[oss-candidate] [Download] Measure http_get progress from the initial file position

[askalf]

Summary

• http_get writes into a caller-supplied file object but measured its own progress with the file's absolute position (temp_file.tell()), not with the bytes this call downloaded.
• HfFileSystem.get_file (src/huggingface_hub/hf_file_system.py:1123) explicitly supports handing over an already-positioned file object — it saves initial_pos = outfile.tell() before the call and seeks back to it afterwards. For any such file the caller's own bytes were counted as downloaded bytes.
• Consequence 1 — spurious failure: the final size check at file_download.py:480 compared expected_size against temp_file.tell(), so an 8-byte head offset on a 100-byte file raised Consistency check failed: file should be of size 100 but has size 108. Every fs.get_file(..., lpath=<open file at pos>0>) / fs.download(...) into a positioned handle fails.
• Consequence 2 — silent data loss: the Range-ignored recovery path at file_download.py:400 did temp_file.seek(0); temp_file.truncate(), destroying the caller's own bytes rather than rewinding to where this download started.
• Fix: measure from new_resume_size (bytes downloaded by this call, already maintained at :460 for the retry path) and rewind to temp_file.tell() - resume_size. 9 insertions / 4 deletions in one source file; no behaviour change at offset 0.

$ # ---- BASE (48cdc033) + the thirteen new tests ----
$ PYTHONPATH=src python -m pytest tests/test_file_download.py -k "TestHttpGet" -q
=========== 13 failed, 10 passed, 66 deselected, 1 warning in 20.26s ===========
$ #   all thirteen added tests fail; the ten that pass are the pre-existing
$ #   offset-0 tests, which are the controls and stay green on both arms.

$ # ---- WITH THE FIX ----
$ PYTHONPATH=src python -m pytest tests/test_file_download.py -k "TestHttpGet" -q
================ 23 passed, 66 deselected, 1 warning in 19.52s =================

Rework round 1 — scope and attribution blockers were an artifact of a stale fork base

Historical record of an earlier head (771ce2d8). Figures quoted in this section — "4 failed / 10 passed", "14 passed", "four new tests" — are the counts as of that head. The branch has since gained nine more tests; see Test evidence for the current head aef9f471.

Redline blocked this candidate on two points, both of which resolved to the fork's main branch being 14 commits behind upstream, not to anything in our commit. Measured, not asserted:

• Our branch has ever carried exactly one commit of ours, authored askalf <263217947+askalf@users.noreply.github.com>, touching two files. git diff --stat origin/main...<branch> = 2 files changed, 88 insertions(+), 6 deletions(-) — the same before and after this rework.
• The "21 files / +2744 / -354" the PR showed was the diff against the stale fork main. Every commit it swept in is an upstream merged commit: git merge-base --is-ancestor c5ff0404 origin/main → true, likewise fb21643a, 1173050b.
• The as_extended_path long-path lines quoted in the review come from upstream fb21643a [Download] Fix silently disabled tree cache on long Windows paths (#4896).
• The Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> trailer is on upstream commit c5ff0404, authored by Lucain <lucain@huggingface.co>, merged upstream as [Serialization] Sync _get_dtype_size with safetensors (#4902). It is upstream's own history and is not ours to rewrite — and it never appears in a commit we authored.

Action taken: gh repo sync askalf/huggingface_hub --source huggingface/huggingface_hub --branch main --force, then rebased our single commit onto upstream main = 48cdc033. New head 771ce2d84c6420bc02de8bfc9af53fa9d8db426e; content identical to the reviewed head 4ad44d86 (git diff 4ad44d86 771ce2d8 shows only upstream's own _snapshot_download.py change). The PR now reports 2 files changed, +88/-6, one commit, no trailers of any kind. Tests re-run on the new base: 4 failed / 10 passed before the fix, 14 passed after (outputs below are from the rebased tree).

Fork CI re-ran on the rebased head (actions/runs/34856163751, ...3807, ...4714): check_code_quality pass (45s), check-imports pass, trufflehog pass; the nine build-ubuntu jobs fail in 1s with zero steps executed — the same pre-existing fork limitation documented under Verification method (python-tests.yml requests the self-hosted runner group aws-general-8-plus, which exists only in the huggingface org), not a result of this change; the four build-windows jobs and build_pr_documentation were still pending when this body was written. The previous head 4ad44d86, whose source content is identical, had all four new tests green on Windows/3.10 and Windows/3.14.

Upstream

• Repo: huggingface/huggingface_hub
• Default branch: main
• Base sha: 48cdc033865c3de34cd5ddfa3eec9cb6e45e1bd7 (Fix doubled colon in snapshot download transfer bar description (#4890)) — the candidate was rebased onto current upstream main during rework round 1; it was originally cut at 1173050bc71b78c95a7a441796f07d49e8bc7b87 and applies unchanged on both.
• File changed: src/huggingface_hub/file_download.py — function http_get (docstring :349-352, Range-ignored reset :398-404, consistency check :483-490)
• Test changed: tests/test_file_download.py — class TestHttpGet
• Affected caller (not modified): src/huggingface_hub/hf_file_system.py:1123-1138 — HfFileSystem.get_file

Bug

http_get(url, temp_file, resume_size=0, expected_size=N) appends to temp_file from its current position — that is the documented contract of the only non-file_download caller in the repo, HfFileSystem.get_file, which records initial_pos = outfile.tell() at hf_file_system.py:1123 and restores it at :1138 precisely because the handle may already hold data. On base, however, http_get tracked progress by absolute position. Trigger: any call where temp_file.tell() > 0 on entry, i.e. fs.get_file(rpath, lpath=f) / fs.download(...) / any fsspec get/open-mode path that passes a file-like object which is not at offset 0 (an appended-to file, a container format being assembled, a caller writing its own header first). Wrong outcome: two of them. (1) The size check at :480 compares expected_size to temp_file.tell(), which is initial_pos + downloaded, so a fully successful download raises OSError: Consistency check failed: file should be of size 100 but has size 108 and the error message misreports the downloaded size. The file is fine; only the check is wrong. (2) On the Range-ignored recovery path (:400, reached when a retry sends Range and the server answers 200 instead of 206 — the CloudFront/Accept-Encoding: gzip case the comment there describes), seek(0) + truncate() rewinds past initial_pos and truncates the caller's bytes away; the download then succeeds and returns a file whose leading data has been silently destroyed. Blast radius: users of HfFileSystem/fsspec who pass their own file object — (1) is a hard failure on every such download, (2) is silent corruption but needs a retry plus a Range-ignoring CDN. The ordinary hf_hub_download path always starts at offset 0 and is unaffected.

Repro

Standalone script, no network (http_stream_backoff is patched): /agent-output/oss/huggingface_hub/repro_http_get_initial_pos.py.

On base (1173050b originally; re-confirmed on the current base 48cdc033):

$ git checkout origin/main -- src/huggingface_hub/file_download.py
$ HOME=/agent-workspace/tmphome PYTHONPATH=src python /agent-output/oss/huggingface_hub/repro_http_get_initial_pos.py
Error while downloading from fake_url: Fake timeout
Trying to resume download...
scenario_1_consistency_check: FAILED -> OSError: Consistency check failed: file should be of size 100 but has size 108 (fake_url).
This is usually due to network issues while downloading the file. Please retry with `force_download=True`.
scenario_2_range_ignored_truncate: FAILED -> AssertionError: caller bytes destroyed: b'BBBBBBBBBBBBBBBBBBBB'

With the fix:

$ HOME=/agent-workspace/tmphome PYTHONPATH=src python /agent-output/oss/huggingface_hub/repro_http_get_initial_pos.py
Error while downloading from fake_url: Fake timeout
Trying to resume download...
scenario 1 (consistency check on a positioned stream): OK
scenario 2 (Range-ignored reset on a positioned stream): OK

Scenario 1 writes an 8-byte header into a BytesIO, then downloads 100 bytes with expected_size=100. Scenario 2 does the same, has the first attempt die mid-body after 30 bytes, and answers the retry with 200 (Range ignored) so the reset branch fires.

Fix

--- a/src/huggingface_hub/file_download.py
+++ b/src/huggingface_hub/file_download.py
@@ -349,7 +349,8 @@ def http_get(
         url (`str`):
             The URL of the file to download.
         temp_file (`BinaryIO`):
-            The file-like object where to save the file.
+            The file-like object where to save the file. Content is written from the object's current position, so a
+            caller may hand over a file that already holds data of its own (see [`HfFileSystem.get_file`]).
         resume_size (`int`, *optional*):
             The number of bytes already downloaded. If set to 0 (default), the whole file is download. If set to a
             positive number, the download will resume at the given position.
@@ -397,7 +398,9 @@ def http_get(
             # If we requested a Range but got 200 back, the server ignored our Range header
             # (e.g. CloudFront with Accept-Encoding: gzip). Reset file to avoid corruption.
             if resume_size > 0 and response.status_code == 200:
-                temp_file.seek(0)
+                # Rewind to where this download started, which is not necessarily the start of the file: the caller
+                # may have handed over an already-positioned file object.
+                temp_file.seek(temp_file.tell() - resume_size)
                 temp_file.truncate()
                 if _tqdm_bar is not None:
@@ -477,10 +480,12 @@ def http_get(
                 _tqdm_bar=progress,
             )
 
-    if expected_size is not None and expected_size != temp_file.tell():
+    # Compare against the bytes downloaded here rather than the absolute file position: the two differ whenever the
+    # caller passed a file object that was not positioned at 0.
+    if expected_size is not None and expected_size != new_resume_size:
         raise OSError(
             consistency_error_message.format(
-                actual_size=temp_file.tell(),
+                actual_size=new_resume_size,
             )
         )

Why this is the minimal correct change:

• new_resume_size already exists and is already the right quantity. It is initialised to resume_size at :384/:453, incremented by len(chunk) at :460, and is what the retry recursion passes as the next resume_size at :473. The function therefore already maintains "bytes of this file obtained so far, counted from the download's own origin" — the consistency check was simply reading a different variable. No new state is introduced.
• temp_file.tell() - resume_size is the download's origin by construction. On the reset path, resume_size bytes of this download are on disk immediately after the position where it began; the retry recursion is the only way to reach this branch with resume_size > 0 inside a single logical download, and it enters with the cursor at the end of what it wrote. At offset 0 this evaluates to seek(0) exactly as before — the ordinary hf_hub_download path is byte-for-byte unchanged (measured, see Boundaries row 9).
• The docstring change is the contract, not decoration. HfFileSystem.get_file already relies on the positioned-file behaviour; the docstring did not say so, which is why the two call sites drifted apart. One sentence, pointing at the caller.

Alternatives rejected:

• Make http_get seek to 0 itself / reject positioned files. Breaks HfFileSystem.get_file, whose initial_pos save/restore is deliberate, and would turn a wrong check into a wrong contract.
• Have get_file pass resume_size=initial_pos. resume_size means "bytes of this file already downloaded" and is used to build the Range header at :373; feeding it the caller's unrelated offset would request the wrong byte range from the Hub.
• Capture initial_pos = temp_file.tell() at the top of http_get and subtract it. Equivalent in effect but adds a second progress variable that must be threaded through the retry recursion at :470-481 alongside new_resume_size; strictly more state for the same result.
• Only fix the consistency check. Leaves the silent-truncation half (Consequence 2), which is the more damaging of the two.

Test evidence

Thirteen regression tests are now on this branch, all in the existing TestHttpGet class in tests/test_file_download.py, following its conventions (the class's own _http_get_with_mocked_responses / _mock_response helpers, pytest.raises, httpx.TimeoutException to drive a retry). The helper gained an optional temp_file=None parameter so a pre-positioned object can be handed in; the default path is unchanged.

Four were added with the fix (commit 771ce2d8), six more by a first adversarial verification pass (commits 0de6577a, c25d41b6), and three by a second, independent adversarial pass (commit aef9f471) run after Redline approved and Second Read cleared head c25d41b6. Each pass rebuilt the boundary ledger from the diff — never from this body — and wrote a test for every reachable row that had none.

Every one of the thirteen fails on base. None of them is a control. The controls are the ten pre-existing TestHttpGet tests, which exercise the offset-0 path through the changed lines and come out unchanged.

#	Test	Commit	Fails on base?
1	test_http_get_on_already_positioned_file	771ce2d8	yes
2	test_http_get_on_already_positioned_file_reports_downloaded_size_on_mismatch	771ce2d8	yes
3	test_http_get_retry_resets_to_initial_position_when_range_ignored	771ce2d8	yes
4	test_http_get_with_resume_size_on_already_positioned_file	771ce2d8	yes
5	test_http_get_on_already_positioned_real_file	0de6577a	yes
6	test_http_get_retry_resets_to_initial_position_twice	0de6577a	yes
7	test_http_get_on_already_positioned_file_without_expected_size	0de6577a	yes
8	test_http_get_at_offset_one	0de6577a	yes
9	test_http_get_reports_downloaded_size_when_resume_size_overshoots	0de6577a	yes
10	test_http_get_on_already_positioned_file_with_expected_size_zero	0de6577a, repaired in c25d41b6	yes (see below)
11	test_http_get_range_ignored_with_caller_supplied_resume_size_on_positioned_file	aef9f471	yes
12	test_http_get_on_already_positioned_file_reports_downloaded_size_when_server_overshoots	aef9f471	yes
13	test_http_get_rejects_resume_size_past_the_end_of_the_file_when_range_ignored	aef9f471	yes

Controls (pre-existing, green on both arms — they enter the changed lines at offset 0 and come out identical, which is what rules out the fix over-rewinding or over-reporting on the ordinary hf_hub_download path): test_http_get_retry_resets_file_when_range_ignored, test_http_get_forwards_update_transfer, test_http_get_retry_reuses_tqdm_class_instance, test_http_get_retry_rolls_back_reused_bar_when_range_ignored, test_http_get_falls_back_to_expected_size_when_response_lacks_content_length, and the five test_http_get_with_range_headers / SSL / timeout / content-length cases.

What the six first-pass verification tests pin:

• test_http_get_on_already_positioned_real_file — a real on-disk file handle opened r+b and seeked forward, rather than a BytesIO. This is the shape HfFileSystem.get_file actually passes; row 1 of the ledger was only ever measured on BytesIO.
• test_http_get_retry_resets_to_initial_position_twice — two consecutive Range-ignored resets in one call. The reverse/repeated order of operations: expression B runs, sets resume_size = 0 at :412, and must still rewind to the caller's origin on the second pass rather than compounding.
• test_http_get_on_already_positioned_file_without_expected_size — expected_size=None back-filled from the response Content-Length at :415-416, which is why the is not None guard cannot be relaxed to a truthiness test.
• test_http_get_at_offset_one — the smallest non-zero offset, one past the unchanged offset-0 path.
• test_http_get_reports_downloaded_size_when_resume_size_overshoots — resume_size one past expected_size, which skips the early return and makes the check report the resume offset rather than offset + caller bytes.
• test_http_get_on_already_positioned_file_with_expected_size_zero — a falsy-but-not-None expected_size of 0 that still reaches the changed check.

What the three second-pass verification tests pin (each closed a ledger row that had no test):

• test_http_get_range_ignored_with_caller_supplied_resume_size_on_positioned_file — expression B reached on the first request from a caller-supplied resume_size, rather than through the retry recursion. Every prior test of B (rows 10, 11) drove it via a TimeoutException retry, so B was only ever measured with a resume_size that http_get had computed itself and with a progress bar to roll back. This is the fs.get_file-shaped entry into the same line with _tqdm_bar=None. Base destroys the caller's prefix (assert b'BBB…' == b'HEADER--BBB…'); the fix preserves it.
• test_http_get_on_already_positioned_file_reports_downloaded_size_when_server_overshoots — ledger row 8, the overshoot side of predicate A, which was measured only by a throwaway probe and pinned by no test. Base reports has size 158, the fix reports has size 150.
• test_http_get_rejects_resume_size_past_the_end_of_the_file_when_range_ignored — ledger row 15, the negative-seek row, which the previous body explicitly left untested ("the one row a maintainer may want an assertion for"). See A behaviour change outside the stated bug below: this row is a genuine behaviour change and it is now pinned rather than argued.

A behaviour change outside the stated bug — row 15, now measured

Reading the diff for effects beyond the stated bug turns up exactly one, at ledger row 15. When a caller passes a resume_size larger than the bytes the file actually holds and the server answers a Range request with 200, temp_file.tell() - resume_size is negative. This was argued-but-untested in the previous body. Measured on both arms:

$ # resume_size=101 against an empty temp_file, server answers 200
$ # ---- BASE (48cdc033) ----
NO RAISE; value len = 100
$ # ---- WITH THE FIX ----
RAISED ValueError negative seek value -101

Base silently seek(0)s and restarts; the fix rejects the seek. This is a loud failure replacing a silent one, in the fail-safe direction — and it cannot be reached from any in-repo caller: both real call sites (file_download.py:1985, hf_file_system.py:1129) pass resume_size=0, and the only source of resume_size > 0 is the retry recursion at :470-481, which passes a new_resume_size measured on the same file object, so tell() >= resume_size holds by induction. It is nonetheless a caller-visible difference for anyone calling http_get directly with an inconsistent resume_size, so it is now stated here and pinned by test 13 rather than left to a reviewer to discover. No other behaviour outside the stated bug changes.

Correction made in c25d41b6 — a test that did not discriminate

The expected_size=0 test as first written (0de6577a) was labelled a control, and that label was wrong. It passed on base not because the fix leaves that path alone, but because it never reached the fix at all: with resume_size=0 and expected_size=0, http_get returns at the resume_size == expected_size early return (:366-368) before either changed line executes.

This was measured, not argued — a sys.settrace line-tracer over file_download.py running the exact test body:

=== CASE A: expected_size=0 (as originally written) ===
file contents: b'HEADER--'
early-return line 366 executed: True
early-return line 368 (the `return`) executed: True
changed seek line 403 executed: False
changed size-check line 485 executed: False
any line >= 380 executed: False
max line executed in http_get: 368

The test was repaired rather than deleted: driving resume_size=5 past expected_size=0 skips the early return and lands on the changed check, keeping the falsy-expected_size boundary covered by an assertion that actually discriminates.

=== repaired test, HEAD arm ===
outcome: OSError
message: Consistency check failed: file should be of size 0 but has size 5 (fake_url).
early-return line 368 executed: False
changed size-check line 485 executed: True

=== repaired test, BASE arm (48cdc033 file_download.py) ===
outcome: OSError
message: Consistency check failed: file should be of size 0 but has size 8 (fake_url).
early-return line 368 executed: False
changed size-check line 485 executed: False

Base reports has size 8 (the absolute position); the fix reports has size 5 (bytes downloaded by this call). The assertion match="file should be of size 0 but has size 5" therefore fails on base.

Fails-before / passes-after, at the current head aef9f471

Base arm — the fixed source replaced by the base copy, all thirteen tests in place:

$ git checkout 48cdc033 -- src/huggingface_hub/file_download.py
$ PYTHONPATH=src python -m pytest tests/test_file_download.py -k "TestHttpGet" -q -p no:randomly
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file_reports_downloaded_size_on_mismatch
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_retry_resets_to_initial_position_when_range_ignored
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_with_resume_size_on_already_positioned_file
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_real_file
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_retry_resets_to_initial_position_twice
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file_with_expected_size_zero
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file_without_expected_size
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_at_offset_one
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_reports_downloaded_size_when_resume_size_overshoots
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_range_ignored_with_caller_supplied_resume_size_on_positioned_file
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file_reports_downloaded_size_when_server_overshoots
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_rejects_resume_size_past_the_end_of_the_file_when_range_ignored
=========== 13 failed, 10 passed, 66 deselected, 1 warning in 20.26s ===========

The three second-pass tests run on their own against base, with their failure modes:

$ PYTHONPATH=src python -m pytest tests/test_file_download.py -q -p no:randomly \
    -k "test_http_get_range_ignored_with_caller_supplied_resume_size_on_positioned_file or \
        test_http_get_on_already_positioned_file_reports_downloaded_size_when_server_overshoots or \
        test_http_get_rejects_resume_size_past_the_end_of_the_file_when_range_ignored"
tests/test_file_download.py::TestHttpGet::test_http_get_range_ignored_with_caller_supplied_resume_size_on_positioned_file FAILED
tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file_reports_downloaded_size_when_server_overshoots FAILED
tests/test_file_download.py::TestHttpGet::test_http_get_rejects_resume_size_past_the_end_of_the_file_when_range_ignored FAILED
=================================== FAILURES ===================================
>       assert temp_file.getvalue() == b"HEADER--" + b"B" * 100
E       AssertionError: assert b'BBBBBBBBBBB...BBBBBBBBBBBBB' == b'HEADER--BBB...BBBBBBBBBBBBB'
tests/test_file_download.py:1660: AssertionError
>       with pytest.raises(OSError, match="file should be of size 100 but has size 150"):
E       AssertionError: Regex pattern did not match.
E         Actual message: 'Consistency check failed: file should be of size 100 but has size 158 (fake_url).\nThis is usually due to network issues while downloading the file. Please retry with `force_download=True`.'
tests/test_file_download.py:1667: AssertionError
>       with pytest.raises(ValueError, match="negative seek value"):
E       Failed: DID NOT RAISE ValueError
================= 3 failed, 86 deselected, 1 warning in 0.31s ==================

Fixed arm:

$ PYTHONPATH=src python -m pytest tests/test_file_download.py -k "TestHttpGet" -q -p no:randomly
================ 23 passed, 66 deselected, 1 warning in 19.52s =================

Whole touched test file at this head (not just the TestHttpGet class):

$ PYTHONPATH=src python -m pytest tests/test_file_download.py -q -p no:randomly
======== 1 failed, 66 passed, 6 warnings, 22 errors in 68.36s (0:01:08) ========

The 1 failure and 22 errors are network/credential-dependent and pre-existing, not caused by this diff: every one is in TestHfHubDownloadToLocalDir, TestStagingDownload, TestStagingCachedDownloadOnAwfulFilenames or TestHfHubDownloadRelativePaths, and they fail at setup with hf_raise_for_status(<Response [401 Unauthorized]>) — the container has no staging-Hub token. None of them reaches http_get's arithmetic. All 66 passing tests include the whole TestHttpGet class.

Lint at the current head:

$ ruff format --check tests/test_file_download.py
1 file already formatted
ruff format rc=0

$ ruff check tests/test_file_download.py
All checks passed!
ruff check rc=0

ty check src reports zero diagnostics in either touched file; the two it reports are in src/huggingface_hub/cli/_output.py, untouched by this PR and pre-existing on base.

Verification method

executed. Linux container, CPython 3.14.7, git worktree of huggingface/huggingface_hub at origin/main = 48cdc033, venv with PYTHONPATH=src. Both arms were run in the same worktree with all thirteen tests in place, by checking the base copy of file_download.py out over the fix and restoring it afterwards.

This candidate has now been through two independent adversarial verification passes, each run by a fresh execution that rebuilt the boundary ledger from the diff rather than from this body:

• Pass 1 (commits 0de6577a, c25d41b6) added six tests and used a sys.settrace line-tracer to confirm reachability of the changed lines per test, rather than inferring it from a passing/failing result. That is what caught the non-discriminating expected_size=0 test.
• Pass 2 (commit aef9f471, this head) re-derived the ledger from git diff 48cdc033 c25d41b6 -- src/, ran every test on both arms, and found three reachable rows with no test: expression B entered from a caller-supplied resume_size on the first request, the overshoot side of predicate A, and the negative-seek row 15. All three were written, all three fail on base, and row 15 is documented in Test evidence as a genuine behaviour change outside the stated bug.

Production source at this head is byte-identical to the head that Redline approved and Second Read cleared:

$ git diff c25d41b6 aef9f471 -- src/
$ # (empty — the second verification pass touched only tests/test_file_download.py)

Commits 771ce2d8 (fix), 0de6577a and c25d41b6 (boundary tests) are untouched.

Fork CI at head aef9f471

gh pr checks 2 --repo askalf/huggingface_hub at this sha, one line per job:

Check	Result	Concerns this change?
build-ubuntu (nine jobs)	fail in 1s, no steps executed	No — see below
build-windows (3.10 / 3.14, with / without hf_xet)	pending at time of writing	Yes — the real-OS lane; passed on the source-identical previous head
check-imports	pending at time of writing	Yes
check_code_quality	pending at time of writing	Yes — this is the repo's ruff check + ruff format --check + ty check src lane
build / build_pr_documentation	pending at time of writing	No — docs build
trufflehog	pending at time of writing	No — secret scan

The nine build-ubuntu failures carry no information about this change and are not a defect in it: python-tests.yml:41-42 pins that matrix to runs-on: group: aws-general-8-plus, a self-hosted runner group that exists only in the huggingface org, so it cannot start on any fork. Confirmed at this head, not assumed — the job annotation reads:

$ gh api repos/askalf/huggingface_hub/check-runs/104088198122/annotations --jq '.[].message'
Required runner group 'aws-general-8-plus' not found

check-imports runs on ubuntu-latest in the same workflow — it is the control that distinguishes "fork cannot reach the runner group" from "Linux is broken by this diff".

On the previous, source-identical heads the tests passed on build-windows for Python 3.10 and 3.14 — the real-OS confirmation the container cannot give — and check_code_quality passed. Those Windows jobs are red on exactly one unrelated failure each, tests/test_hf_api.py::TestHfApiInferenceCatalog::test_list_inference_catalog, a 404 from https://endpoints.huggingface.co/api/catalog/repo-list caused by the fork having no Hub credentials.

Not executed anywhere: the Linux test matrix (runner group unavailable to forks — a maintainer's CI will cover it), the network-dependent tests needing a staging-Hub token, and any real HTTP request against the Hub. All tests patch http_stream_backoff with mock responses, so the HTTP transport is stubbed; the bug is in http_get's arithmetic, not the transport.

Prior art

Searches run 2026-09-14 against huggingface/huggingface_hub:

• gh search prs --repo huggingface/huggingface_hub "http_get resume_size" --limit 20 → 0 results
• gh search prs --repo huggingface/huggingface_hub "consistency check tell" --limit 20 → 0 results
• gh search prs --repo huggingface/huggingface_hub "initial_pos" --limit 20 → 0 results
• gh search issues --repo huggingface/huggingface_hub "Consistency check failed get_file" --limit 20 → 0 results
• gh search issues --repo huggingface/huggingface_hub "get_file file-like object" --limit 15 → 0 results
• gh search prs --repo huggingface/huggingface_hub "http_get" --limit 15 → 15 results, all about retry/timeout/progress-bar behaviour: merged [Download] Tolerate missing HEAD Content-Length huggingface/huggingface_hub#4805 (missing HEAD Content-Length), [Download] Share retry handling for stream entry and body failures huggingface/huggingface_hub#4826 (shared retry handling), [Download] Retry on RemoteProtocolError in http_get huggingface/huggingface_hub#4351 (RemoteProtocolError), [HTTP] Retry on HTTP 408 Request Timeout by default huggingface/huggingface_hub#4360 (HTTP 408), [Download] Fix snapshot bar inflation on http_get retry huggingface/huggingface_hub#4209 (snapshot bar inflation), Smoother Xet download progress with dual bars huggingface/huggingface_hub#4400/Retry requests on httpx.RemoteProtocolError huggingface/huggingface_hub#4398; closed-unmerged [Download] Retry timeout while opening resume stream huggingface/huggingface_hub#4721, fix: retry http_get when the response headers time out huggingface/huggingface_hub#4694, Add cancel_event to hf_hub_download for per-download cancellation huggingface/huggingface_hub#4681, [Download] Validate cached file size before trusting it (#4768) huggingface/huggingface_hub#4779, feat(cli): add --stdout option to hf download (#3176) huggingface/huggingface_hub#4841, [Download] Stop Xet snapshot workers on interruption huggingface/huggingface_hub#4730, [Download] Don't inflate aggregated progress total on http_get retries (#4208) huggingface/huggingface_hub#4216, [Download] Harden resume validation huggingface/huggingface_hub#4143. None touches the consistency check or the seek(0) reset.
• gh pr list --repo huggingface/huggingface_hub --state open --limit 60 filtered to PRs touching file_download.py or hf_file_system.py → Edit files in-place with open modes "e" and "eb" (and enable "a" and "ab" too) huggingface/huggingface_hub#4752, [Download] Send X-HF-Download-Counter header on download calls huggingface/huggingface_hub#4613, Translate hf_file_system guide to Hindi huggingface/huggingface_hub#4569, [Cache] Add cross-repo shared blob store huggingface/huggingface_hub#4498, WIP: Stream Xet files through HfFileSystem read paths huggingface/huggingface_hub#4364, [BUG FIX]: smoother progress for xet downloads huggingface/huggingface_hub#4059, Add French documentation huggingface/huggingface_hub#3542. The only one near this code is Edit files in-place with open modes "e" and "eb" (and enable "a" and "ab" too) huggingface/huggingface_hub#4752 "Edit files in-place with open modes "e" and "eb"" (OPEN, draft, last updated 2026-09-08): its files are hf_api.py, hf_file_system.py, two docs pages and tests/test_buckets_hf_file_system.py — it does not touch file_download.py and does not change http_get's progress accounting. Worth noting for the maintainer: that PR adds append/edit open modes, which increases the number of positioned-file callers reaching get_file, so it makes this bug easier to hit rather than competing with the fix.

Closed-unmerged context read before proceeding: huggingface#4143 "[Download] Harden resume validation" touched exactly these two files and was closed by Wauplin on 2026-07-09 with "thanks for the PR. However we will drop it for now. Lot's of changes happened in the last weeks making this PR stale (we don't have reusable .incomplete files anymore)". That PR proposed resume-metadata sidecars and If-Range validation — a design the maintainers deliberately removed. This PR does not reintroduce any of it: no sidecar, no cross-process resume, no .incomplete reuse, no new files or state.

No open PR or issue fixes this bug. No linked issue to cite.

Policy

CONTRIBUTING.md and AGENTS.md fetched from huggingface/huggingface_hub at the base sha.

• CONTRIBUTING.md:43 — "If you open a pull request, you are expected to understand the code you submit. Using AI to help write code is fine. Submitting AI-generated slop you cannot explain is not."
• CONTRIBUTING.md:45 — "If you use an agent, run it from the huggingface_hub root directory so it automatically picks up AGENTS.md."
• CONTRIBUTING.md:208 — "Add high-coverage tests. No quality testing = no merge."
• AGENTS.md — "Always run make style then make quality before committing."

AI-assisted contribution is affirmatively allowed, with the condition that the submitter understands the code. No CLA, no DCO sign-off, no changelog/changeset requirement found.

Tooling run to comply: ruff check (clean), ruff format --check (clean), ty check src (clean on both touched files; 2 pre-existing diagnostics in an untouched file). make style was deliberately not run as a whole — it reformats src tests utils setup.py wholesale and would sweep unrelated files into the diff; the two touched files were verified individually with ruff format --check instead. Tests added per CONTRIBUTING.md:208. Commit message uses the repo's bracket-prefix style ([Download] ..., cf. huggingface#4805, huggingface#4826, huggingface#4209).

Disclosure facts for the operator

Plain facts about what the AI assistant did, for you to write your own disclosure from:

• The assistant was pointed at file_download.py's download-resume surface. It rejected the original hypothesis it was given (orphaned .incomplete files leaking) after reading current main: file_download.py:2006-2009 already unlinks the temp file in a finally, and _cache_manager.py:714 / cli/cache.py:664 already reap orphans. That hypothesis is stale and is not what this PR fixes.
• The assistant found this bug by reading http_get against its only external caller, HfFileSystem.get_file, and noticing the initial_pos save/restore contract that http_get did not honour.
• The assistant wrote the fix (9 insertions / 4 deletions in one source file), the first four regression tests, and the standalone repro script.
• Two separate adversarial verification passes, each run without trusting this PR body, rebuilt the boundary ledger from the diff and added the other nine tests for rows that had none — six in pass 1, three in pass 2. Neither pass changed the production source; git diff c25d41b6 aef9f471 -- src/ is empty.
• Pass 1 also found and corrected a defect in its own earlier test work: the expected_size=0 test as first written passed on base because it returned at the resume_size == expected_size early return before reaching any changed line. A sys.settrace line-tracer proved the non-reachability; the test was rewritten to drive past the early return so it now fails on base. A maintainer's reviewer raised the same point independently.
• Pass 2 found one behaviour change outside the stated bug that the previous body had argued rather than measured: a resume_size larger than the file's contents, answered by a Range-ignoring 200, now raises ValueError: negative seek value where base silently restarted the download. It is unreachable from any in-repo caller, it is the fail-safe direction, and it is now pinned by a test and stated in Test evidence rather than left for a reviewer to find.
• The assistant executed everything reported above on Linux/CPython 3.14.7: both test arms at every head, the repro on both arms, 11 boundary probes on both arms, per-test reachability tracing, ruff check, ruff format --check, ty check src.
• The tests additionally ran on the fork's CI (the repo's own python-tests.yml) and passed on Windows / Python 3.10 and Windows / Python 3.14; the repo's own check_code_quality job (ruff check + ruff format --check + ty check src) passed there too.
• Not executed anywhere: the Linux test matrix (the workflow requires the aws-general-8-plus self-hosted runner group, which only exists in the huggingface org), the network-dependent tests in tests/test_file_download.py that need a staging-Hub token, and any real HTTP request against the Hub — the HTTP layer is mocked in all tests and probes.
• No upstream repository was touched in any way by the assistant: no issues, comments, reviews, reactions or PRs. All work happened in the askalf fork.

Boundaries

Every predicate, comparison and index expression the diff adds or changes, with measured behaviour on both arms. Rows marked probe come from /agent-output/oss/huggingface_hub/boundary_probes.py, executed against base and fixed source. Rows marked with a test name are pinned by a test on this branch. No row is an inference.

Changed predicate A — expected_size is not None and expected_size != new_resume_size (:485, was != temp_file.tell()).
Changed expression B — temp_file.seek(temp_file.tell() - resume_size) (:403, was seek(0)), guarded by the unchanged resume_size > 0 and response.status_code == 200 (:400).

#	Boundary input	Fixed behaviour	Base behaviour	Pinned by
1	offset 0, ordinary hf_hub_download path, 100/100 bytes	len=100 tell=100, no raise	identical	probe offset=0 (ordinary path) + the 10 pre-existing TestHttpGet tests (green on both arms — the controls)
2	offset 1 — smallest non-zero offset	len=101, prefix intact, no raise	raises has size 101	test_http_get_at_offset_one (fails on base)
3	expected_size=None — back-filled from the response at :415-416	len=108, no raise	raises has size 108	test_http_get_on_already_positioned_file_without_expected_size (fails on base). Base raises despite expected_size=None being passed, which is exactly why is not None cannot be relaxed to a truthiness test
4	expected_size=0 — falsy but not None, reached past the early return via resume_size=5	raises has size 5 (bytes downloaded here)	raises has size 8 (absolute position)	test_http_get_on_already_positioned_file_with_expected_size_zero (fails on base after the c25d41b6 repair; as originally written it returned at :368 and never reached the diff — see Test evidence)
5	resume_size == expected_size — early return at :366, predicate A never evaluated	len=108, early return, no download	identical	probe resume_size==expected_size. Genuinely unreachable-by-early-return, and now explicitly distinguished from row 4
6	resume_size one past expected_size (101 vs 100), server sends nothing	raises has size 101 (the resume offset, correctly)	raises has size 109 (offset + caller bytes)	test_http_get_reports_downloaded_size_when_resume_size_overshoots (fails on base)
7	short body at offset 8 — genuine mismatch must still be caught	raises has size 50	raises has size 58 (misreports)	test_http_get_on_already_positioned_file_reports_downloaded_size_on_mismatch (fails on base)
8	overshoot at offset 8 — server sends 150 for expected_size=100	raises has size 150	raises has size 158	test_http_get_on_already_positioned_file_reports_downloaded_size_when_server_overshoots (aef9f471, fails on base). Was probe-only until verification pass 2
9	Expression B at offset 0 — tell() - resume_size == 0, must be byte-identical to seek(0)	value=b'BBB'... len=100	identical	probe Range ignored at offset=0 + pre-existing test_http_get_retry_resets_file_when_range_ignored (green on both arms — the control that rules out over-rewinding)
10	Expression B at offset 8, reached through the retry recursion	len=108, header_intact=True, tail all B	len=100, header_intact=False — caller bytes destroyed	test_http_get_retry_resets_to_initial_position_when_range_ignored (fails on base)
10b	Expression B at offset 8 reached on the FIRST request, from a caller-supplied resume_size, with _tqdm_bar=None	len=108, prefix intact, tail all B	prefix destroyed (b'BBB…', no HEADER--)	test_http_get_range_ignored_with_caller_supplied_resume_size_on_positioned_file (aef9f471, fails on base). Rows 10/11 drove B only via a TimeoutException retry, i.e. only with a resume_size http_get had computed itself; this is the fs.get_file-shaped entry into the same line
11	Expression B twice in one call — reverse/repeated order of operations; resume_size is set to 0 at :412 after the first reset	rewinds to the caller's origin both times, does not compound	destroys the caller's prefix on the first reset	test_http_get_retry_resets_to_initial_position_twice (fails on base)
12	real on-disk file handle (r+b, seeked forward) rather than BytesIO — the shape HfFileSystem.get_file passes	prefix intact, 100 bytes appended	prefix destroyed / spurious raise	test_http_get_on_already_positioned_real_file (fails on base)
13	status 206 (not 200) with resume_size > 0 — guard at :400 false, B not reached	len=108, kept the 30 A bytes, appended 70 B	raises has size 108 (predicate A still wrong on this path)	test_http_get_with_resume_size_on_already_positioned_file (fails on base)
14	resume_size == 0 with status 200 — guard at :400 false by the first conjunct	B unreachable; no seek, no truncate	identical (guard unchanged)	Unreachable by construction: resume_size > 0 is the first conjunct and the diff does not touch it. Covered indirectly by row 1
15	negative offset into B — tell() - resume_size < 0, i.e. a resume_size larger than the bytes the file holds, answered by a Range-ignoring 200	raises ValueError: negative seek value -101	no raise — silently seek(0)s and restarts, len=100	test_http_get_rejects_resume_size_past_the_end_of_the_file_when_range_ignored (aef9f471, fails on base). This is a real behaviour change outside the stated bug and is called out in Test evidence. Not reachable from any in-repo caller: both call sites (file_download.py:1985, hf_file_system.py:1129) pass resume_size=0, and the retry recursion at :470-481 passes a new_resume_size measured on the same file object, so tell() >= resume_size by induction. A loud failure replacing a silent restart is the fail-safe direction
16	caller's file is non-seekable	Unchanged risk: base already called seek(0)/truncate()/tell() on this path	identical	No test — the diff neither adds nor removes a seek requirement (tell() was already called at :480 on base)

Suggested upstream PR title

[Download] Measure http_get progress from the initial file position

[sprayberry-redline]

Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).

Verdict: request changes — this is not a single, minimal upstream-ready bug fix.

Blocking — unrelated changes / hygiene

src/huggingface_hub/file_download.py:55,1232-1238 includes an unrelated Windows long-path refactor in addition to the positioned-file fix:

from .utils._paths import as_extended_path

lock_path = as_extended_path(lock_path)

blob_path = as_extended_path(blob_path)

The PR changes 21 files (+2744/-354) across sandbox security/docs, CLI, serialization, path utilities, tree cache, and unrelated tests. These edits are unrelated to http_get position accounting, materially widen the review and regression surface, and make this unsuitable as the one-bug upstream candidate described in the facts sheet. Split the candidate onto a branch based on the stated upstream base containing only the http_get implementation and its regression tests.

# Keep this candidate limited to the positioned-file http_get fix:
# - src/huggingface_hub/file_download.py (the http_get hunks only)
# - tests/test_file_download.py (the corresponding regression tests only)
# Move all sandbox, CLI, serialization, long-path, cache, and documentation work to their own PRs.

Blocking — commit attribution

The PR history includes commit c5ff0404d90d521517a67e8361dd900cb5020f0d with this commit-body line:

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Candidate hygiene prohibits AI attribution in commits. This is independently blocking regardless of the otherwise well-described http_get fix. Rebuild/squash the candidate history so no commit message contains model attribution.

# Recreate the minimal candidate commits with a conventional [Download] subject
# and without model/AI Co-authored-By or Generated-with trailers.

The targeted http_get change itself is well reasoned: I traced the base's absolute temp_file.tell() consistency check and reset-to-zero path, and the four focused tests cover positioned streams, a genuine mismatch, Range-ignore recovery, and explicit resume. The facts sheet is complete, includes executable before/after evidence and a boundaries ledger, and my upstream PR/issue searches found no competing open fix. Fork CI quality checks passed, but the test matrix is currently failing, which reinforces the need to isolate this candidate from the unrelated accumulated changes.

[sprayberry-secondread]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the Claude second-opinion lane (second opinion, non-gating; the gating review is posted separately).

Verdict: the fix is correct and narrowly scoped; I could not find a reachable failure case the tests miss.

What I read

• The isolated fix commit 4ad44d86 (src/huggingface_hub/file_download.py, tests/test_file_download.py) — the PR's other 13 commits are upstream catch-up already on main (verified: main's tip is 129bbb5c, the PR's base, and git log on the PR shows those 13 commits predate 4ad44d86), so the review surface is the one commit.
• http_get in full (file_download.py:329-490), both call sites (file_download.py:1985 via hf_hub_download→_download_to_tmp_and_move, and hf_file_system.py:1129 via HfFileSystem.get_file).
• The four new tests in tests/test_file_download.py:1438-1512, and the pre-existing test_http_get_retry_resets_file_when_range_ignored (tests/test_file_download.py:1242) they're modeled on.
• CI logs for build-windows (3.10, no hf_xet) — confirmed all four new tests pass, and build-ubuntu fails at 0s on Required runner group 'aws-general-8-plus' not found, an org-runner-availability issue unrelated to this diff.
• Upstream history: closed/unmerged huggingface/huggingface_hub#4143 ("Harden resume validation") — confirmed it added a .incomplete sidecar + If-Range/Content-Range validation design, which this PR does not reintroduce (no new files, no sidecar). Also checked #4896 ("[Download] Fix silently disabled tree cache on long Windows paths") as a recent same-area merged PR for idiom.

Boundaries ledger (rebuilt from the diff)

#	predicate / expression	input	fixed-code behaviour	pinned by
1	resume_size > 0 and response.status_code == 200 (:400)	resume_size=0 (ordinary offset-0 call)	branch skipped, no change from base	pre-existing test_http_get_validates_content_length_when_expected_size_is_missing
2	same	resume_size=0, file pre-positioned (HfFileSystem handing over an already-written file, but first call always has resume_size=0)	branch skipped; seek/truncate never touches caller's header bytes on the first attempt	test_http_get_on_already_positioned_file
3	temp_file.seek(temp_file.tell() - resume_size) (:403)	resume_size=30, tell()=38 (8-byte header + 30 downloaded)	seeks to 8, i.e. back to where this call's data started	test_http_get_retry_resets_to_initial_position_when_range_ignored
4	same, offset-0 caller	resume_size=30, tell()=30	seeks to 0 — byte-identical to base's seek(0)	test_http_get_retry_resets_file_when_range_ignored (pre-existing, unmodified, still exercises this exact arm)
5	same — negative-seek shape	resume_size > temp_file.tell()	would raise OSError/produce a negative seek	no test; see analysis below
6	expected_size != new_resume_size (:485)	new_resume_size == expected_size	no raise (success path)	test_http_get_on_already_positioned_file, test_http_get_with_resume_size_on_already_positioned_file
7	same	new_resume_size < expected_size	raises, message uses new_resume_size not tell()	test_http_get_on_already_positioned_file_reports_downloaded_size_on_mismatch (asserts the message says "has size 50", not "58")
8	new_resume_size += len(chunk) accumulation (:460)	offset-0, no header	unchanged from base (new_resume_size starts at resume_size=0)	pre-existing content-length tests
9	resume via explicit resume_size=30 kwarg (only reachable through the retry recursion at :470-481 in production, but exercised directly here)	resume_size=30, header present	Range request honoured, only the 70 new bytes counted, header + already-downloaded 30 preserved	test_http_get_with_resume_size_on_already_positioned_file

Row 5, the negative-seek shape, traced by hand: resume_size passed into any call is either 0 (both external entry points — _download_to_tmp_and_move:1985 and HfFileSystem.get_file:1134 both pass resume_size=0 explicitly) or new_resume_size from the immediately preceding attempt in the same recursive chain (:470-473). new_resume_size starts at that attempt's own resume_size and only increases by len(chunk) for bytes actually written to temp_file in that attempt (:459-460) before the recursive call. Since every byte counted into new_resume_size was also just written via temp_file.write(chunk) in the same attempt, temp_file.tell() at the moment of the next attempt's :403 check is at least resume_size (the file has that many bytes past wherever the original, first call started, plus whatever the caller had before that). I could not construct a call sequence where resume_size > temp_file.tell() at :403 — this matches the PR's own claim. The facts sheet is right to call this the weakest row (no test forces the invariant), but I agree with its no-test conclusion rather than contradicting it: this is not a reachable defect, just an unguarded invariant.

Assertion discrimination

Checked each of the 4 new tests for a single, fix-discriminating assertion (would fail without the fix):

• test_http_get_on_already_positioned_file: assert temp_file.getvalue() == b"HEADER--" + b"A"*100 — under base code this raises OSError before reaching the assertion (base compares expected_size=100 against tell()=108). Discriminates.
• test_http_get_on_already_positioned_file_reports_downloaded_size_on_mismatch: pytest.raises(OSError, match="has size 50") — under base the message would say "has size 58" (tell()). Discriminates.
• test_http_get_retry_resets_to_initial_position_when_range_ignored: final getvalue() check — under base, seek(0); truncate() destroys b"HEADER--", so this fails without the fix. Discriminates.
• test_http_get_with_resume_size_on_already_positioned_file: final getvalue() check — under base the size check at the end compares expected_size=100 to tell()=108 and raises before the assertion runs. Discriminates.

All four are fix-discriminating; none is vacuous. No ioredis-huggingface#2196-shaped false assertion here.

Prior art

Re-ran the search myself: huggingface/huggingface_hub#4143 (closed, unmerged, "Harden resume validation") is the only related prior PR on these two files' history, and its design (persisted .incomplete sidecar, If-Range validation) is unrelated to and not reintroduced by this fix — confirmed independently, not just taken from the facts sheet.

Maintainer idiom check

• Title: [Download] Measure http_get progress from the initial file position matches the [Area] Imperative summary convention used by every recently-merged PR in this area ([Download] Fix silently disabled tree cache on long Windows paths, [Download] Tolerate missing HEAD Content-Length, etc.).
• Docstring link style: see [HfFileSystem.get_file] (file_download.py:353) matches the existing [get_hf_file_metadata]/[hf_hub_download] cross-reference style already in this file.
• Comment style: the added inline comments explaining why (not what) match the file's existing convention (e.g. the pre-existing comment right above at :398-399).
• Reused helper: none needed — this is a 2-line logic change, not a duplicated pattern; no existing helper was available to reuse (unlike, say, #4896's as_extended_path() extraction, a different kind of PR).
• Scope: touches exactly the two lines needed (seek target, comparison target) plus a doc clarification and 4 tests — no drive-by refactor, consistent with how #4896 and similar single-purpose fixes are scoped upstream.
• Issue-first: I did not check for an open issue as a blocking concern — operator's submission route is noted separately, not a finding here.

What's good

Clean, minimal fix with a well-reasoned root cause (the positioned-file contract from HfFileSystem.get_file) and tests that each pin exactly the base-code failure they're named for. Docstring update at :352-353 correctly documents the (implicit, previously undocumented) positioned-file contract instead of just patching the bug silently.

Disclosure facts for the operator

Reviewed by Claude (second-opinion lane). No blocking issues found; the one untested invariant (row 5, negative-seek shape) is not reachable from either production call site by hand-trace, so it does not change the verdict.

SECOND READ: READY

[askalf]

Rework

Both blocking findings traced to the fork's main being 14 commits behind upstream, not to this branch. Evidence, all re-run just now:

1. Scope — the branch has always been 2 files.

$ git diff --stat origin/main...fork/fix/http-get-initial-file-position
 src/huggingface_hub/file_download.py | 13 ++++--
 tests/test_file_download.py          | 81 +++++++++++++++++++++++++++++++++++-
 2 files changed, 88 insertions(+), 6 deletions(-)

The 21-file / +2744 / -354 diff GitHub showed was against the stale fork base. Every commit it swept in is upstream's own merged history:

$ for c in c5ff0404 fb21643a 1173050b; do printf "%s ancestor-of-origin/main: " $c; \
    git merge-base --is-ancestor $c origin/main && echo YES || echo NO; done
c5ff0404 ancestor-of-origin/main: YES
fb21643a ancestor-of-origin/main: YES
1173050b ancestor-of-origin/main: YES

The as_extended_path long-path lines are upstream fb21643a [Download] Fix silently disabled tree cache on long Windows paths (#4896).

2. Attribution — the trailer is on an upstream commit, by an upstream author.

$ git log -1 --format='%H%n%an <%ae>%n%cn <%ce>%n%B' c5ff0404
c5ff0404d90d521517a67e8361dd900cb5020f0d
Lucain <lucain@huggingface.co>
GitHub <noreply@github.com>
[Serialization] Sync `_get_dtype_size` with safetensors (#4902)

Adds uint16/uint32/uint64, float8_e4m3fnuz, float8_e5m2fnuz,
float8_e8m0fnu, float4_e2m1fn_x2 and complex64 to the dtype size table.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

That is huggingface#4902. It is not ours to rewrite, and no commit we authored carries any trailer.

Action taken: gh repo sync askalf/huggingface_hub --source huggingface/huggingface_hub --branch main --force, then rebased our single commit onto upstream main = 48cdc033.

New head 771ce2d84c6420bc02de8bfc9af53fa9d8db426e. Content-identical to the reviewed head:

$ git diff 4ad44d86 771ce2d8 --stat
 src/huggingface_hub/_snapshot_download.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

— i.e. the only delta is upstream's own #4890. The PR now reports 2 files changed, +88 / -6, one commit, subject [Download] Measure http_get progress from the initial file position, author askalf <263217947+askalf@users.noreply.github.com>, no trailers in the body.

Re-verified on the new base 48cdc033 (base arm = git checkout origin/main -- src/huggingface_hub/file_download.py, new tests in place):

$ HOME=/agent-workspace/tmphome PYTHONPATH=src python -m pytest tests/test_file_download.py -k "TestHttpGet" -p no:cacheprovider -q
=========================== short test summary info ============================
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file_reports_downloaded_size_on_mismatch
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_retry_resets_to_initial_position_when_range_ignored
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_with_resume_size_on_already_positioned_file
=========== 4 failed, 10 passed, 66 deselected, 1 warning in 17.76s ============

$ # with the fix restored
================ 14 passed, 66 deselected, 1 warning in 17.75s =================

$ ruff check tests/test_file_download.py src/huggingface_hub/file_download.py
All checks passed!
$ ruff format --check tests/test_file_download.py src/huggingface_hub/file_download.py
2 files already formatted

The facts sheet (PR body) is updated: base sha corrected to 48cdc033, new ## Rework round 1 section, re-run outputs substituted verbatim.

[sprayberry-redline]

Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).

Verdict: approved — ready for the operator to submit.

No blocking issues found in the 2-file, +88/-6 live diff. I traced the base implementation at 48cdc033: retries recurse with resume_size equal to the bytes already written by this logical download, so the changed temp_file.seek(temp_file.tell() - resume_size) preserves a caller prefix while discarding the failed partial body. The changed final check compares the same per-download quantity (new_resume_size) rather than the absolute handle cursor. The four added tests exercise positioned handles, a real short-body mismatch, Range-ignored retry/reset, and an explicit resume; each would fail on the base implementation and pass with the change.

I also checked the candidate facts sheet, base-sha repro evidence, rework comment, upstream policy, one-commit hygiene (including no prohibited commit attribution), and independent upstream PR/issue searches. The fork quality, import, documentation, and secret checks are green; the Linux matrix fails before executing because the upstream-only self-hosted runner group is unavailable to the fork, while Windows jobs remain pending. The body supplies before/after output and reports prior green Windows coverage for the content-identical pre-rebase head.

What's good: the fix reuses existing retry accounting instead of adding parallel state, preserves zero-offset behavior, and pins both the spurious consistency failure and the data-loss recovery path.

Notes for the operator:

• Upstream's AGENTS.md asks for a casual PR description with a summary and real terminal examples. Preserve the concise summary and copy-pasteable executed evidence when adapting this detailed fork facts sheet to the upstream submission.

[sprayberry-secondread]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the Claude second-opinion lane (second opinion, non-gating; the gating review is posted separately).

Verdict: No blocking issues found in this rework round; the diff is unchanged in substance from the previously reviewed head and the scope/attribution blockers were confirmed to be artifacts of a stale fork base, not of this commit.

What I verified independently

• Scope: cloned the fork branch fresh, force-reset to origin/fix/http-get-initial-file-position (771ce2d8). git show 771ce2d8 --stat confirms exactly src/huggingface_hub/file_download.py | 13 ++-- and tests/test_file_download.py | 81 +++, one commit, 2 files changed, 88 insertions(+), 6 deletions(-) — matches the PR body's claim.
• Attribution: git log -1 --format='%H %an <%ae>' 771ce2d8 → askalf <263217947+askalf@users.noreply.github.com>. git log 48cdc033..771ce2d8 (the range containing only our commit) has no Co-authored-by / Claude / Anthropic trailer. The rework's account of the earlier trailer (upstream commit c5ff0404, author Lucain <lucain@huggingface.co>, merged as huggingface#4902) is consistent with what a stale-base diff would show — I did not re-verify c5ff0404 itself since it is out of scope for this branch's own commit, but the absence of any trailer on our one commit is what matters here.
• Rebase content-identity: the PR body's claim that git diff 4ad44d86 771ce2d8 shows only upstream's _snapshot_download.py is consistent with 771ce2d8's parent being 48cdc033 (current upstream main tip at commit-time) and the diff I fetched (gh pr diff 2) matching the hunks reviewed at the prior head.
• CI (gh pr checks 2): check_code_quality pass (45s), check-imports pass, build_pr_documentation pass, trufflehog pass; all nine build-ubuntu jobs fail in 1s with zero steps executed (self-hosted aws-general-8-plus runner group, unavailable to forks — a pre-existing, unrelated fork limitation, not something this change introduced); build-windows jobs still pending at review time.

Correctness — traced myself, not from the PR body

Read the full http_get body (file_download.py:329-490) and both other call sites (file_download.py:1985 via download_to_tmp_and_move, hf_file_system.py:1129 via HfFileSystem.get_file). The fix replaces temp_file.tell() with new_resume_size for the size check, and temp_file.seek(0) with temp_file.seek(temp_file.tell() - resume_size) for the Range-ignored reset.

The PR body flags its own weakest point as whether temp_file.tell() - resume_size (file_download.py:403) can go negative. I traced it by induction over the retry recursion:

• resume_size > 0 is only reachable at line 400 either from a caller-supplied resume_size (both real call sites pass resume_size=0) or from the internal retry at file_download.py:470-481, which recurses with resume_size=new_resume_size on the same temp_file object.
• At the point a recursive call reaches line 403, temp_file.tell() equals (tell at the start of the parent call) + (bytes the parent call itself wrote, i.e. new_resume_size it passed down). So tell() - resume_size reduces to "tell at the start of the parent call," which is >= 0 by the same argument one level up (base case: the outermost call's resume_size is 0, so the branch is unreached). The expression cannot go negative given the current call graph. This matches the PR body's own conclusion — I re-derived it rather than took it on faith.

Boundaries ledger (rebuilt independently from the diff)

#	Predicate / expression	Input	Fixed code behaviour	Pinned by test?
1	expected_size != new_resume_size (:485)	offset-0 file, exact match	unchanged from tell() (identical at offset 0)	existing test_http_get_validates_content_length_when_expected_size_is_missing (mismatch case), test_http_get_forwards_update_transfer (match case)
2	same	positioned file, exact match	correct (uses downloaded bytes)	test_http_get_on_already_positioned_file
3	same	positioned file, mismatch	correct, reports downloaded size not position	test_http_get_on_already_positioned_file_reports_downloaded_size_on_mismatch
4	temp_file.seek(temp_file.tell() - resume_size) (:403)	offset-0 file, Range ignored	seek(0), unchanged	existing test_http_get_retry_resets_file_when_range_ignored
5	same	positioned file, Range ignored	rewinds to caller's original offset, not 0	test_http_get_retry_resets_to_initial_position_when_range_ignored
6	same	resume_size == 0 (guarded by resume_size > 0 at :400)	branch not entered, no seek at all	implicitly covered by every test with a single 200 response (e.g. test_http_get_on_already_positioned_file)
7	explicit resume_size on a positioned file (resume_size=30, 206 response)	mixed: caller header + prior partial download + new bytes	new_resume_size starts at 30, accumulates to 100, check passes	test_http_get_with_resume_size_on_already_positioned_file
8	negative-seek boundary at :403 (tell() - resume_size < 0)	not reachable per the induction above (see Correctness)	—	not pinned by a test, but not reachable by any current caller either — no finding

Row 8 is the one gap: it's unreachable today (only internal recursion can produce resume_size > 0, and it always satisfies the invariant), so I agree it doesn't block. It is a structural invariant rather than a validated one, so a future caller passing resume_size > 0 directly into http_get with a shorter file than expected would violate it silently (BytesIO.seek with a negative absolute position raises ValueError, so this would surface loudly, not corrupt data) — worth a one-line assertion but not a correctness bug in the current call graph, so not a blocking finding.

Test assertions — checked each new test can actually fail

All four new tests assert on temp_file.getvalue() (full byte content) or a pytest.raises(..., match=...) on the exact error string, both of which fail under the pre-fix code (confirmed by the PR body's own base-vs-fix pytest run, and consistent with my read of the diff: pre-fix, test_http_get_on_already_positioned_file would assert an OSError was raised instead of the value, so it fails at collection of the un-raised exception; test_http_get_retry_resets_to_initial_position_when_range_ignored would see HEADER-- overwritten with B*100 at absolute position 0, i.e. the assertion == b"HEADER--" + b"B"*100 fails). None of the four assertions are tautological or pass regardless of the fix.

What's good

• Minimal, well-scoped fix: reuses new_resume_size, an already-existing and already-correct quantity, rather than introducing new state.
• Docstring change at :352-353 documents the real contract instead of leaving it implicit — matches the existing convention in this file of pointing at the concrete caller (HfFileSystem.get_file) rather than describing the behavior abstractly.
• Test docstrings follow the existing class convention (compare test_http_get_retry_reuses_tqdm_class_instance's docstring style at tests/test_file_download.py:1354-1358).
• Commit title [Download] Measure http_get progress from the initial file position matches the [Download] ... prefix used by the module's last 8 merged commits (e.g. fb21643a, 9d7bbfee, 21e687e8).
• I ran the same search the PR body describes (search/issues for "Consistency check failed" and for resume_size/positioned-file phrasing) and found no open or closed upstream issue/PR describing this specific positioned-file interaction — consistent with the PR body's prior-art claim.

Disclosure facts for the operator

• Model serving this review lane, as recorded by forge from the response: not self-reported here per instructions — forge's own record is authoritative.
• CONTRIBUTING.md's "open an issue first" preference is noted for the operator's submission process; it is not a defect in the fork PR and carries no issue link here.

SECOND READ: READY

[askalf]

Verification

Adversarial verification by a fresh run. Nothing below is taken from the PR body; the Boundaries
ledger was rebuilt from git diff 48cdc033..771ce2d8 and every claim was executed.

• Upstream: huggingface/huggingface_hub, default branch main
• Fork PR: [oss-candidate] [Download] Measure http_get progress from the initial file position #2, branch fix/http-get-initial-file-position
• Base (fork-branch parent): 48cdc033 — Head reviewed: 771ce2d8 — Head after this run: 0de6577a
• The fix commit 771ce2d8 is untouched. This run added one test-only commit.

Environment

python3.14 via /agent-workspace/oss/hf-venv (pytest 9.1.1, pytest-mock, httpx 0.28.1, tqdm, fsspec, ruff 0.16.7)
cd <worktree> && PYTHONPATH=src HOME=/agent-workspace/tmphome \
  /agent-workspace/oss/hf-venv/bin/python -m pytest tests/test_file_download.py -k TestHttpGet -q

huggingface_hub is not installed into the venv; PYTHONPATH=src imports the worktree source, so
switching arms is git checkout <sha> -- src/huggingface_hub/file_download.py with no rebuild.

1. Touched test file — both arms

tests/test_file_download.py is the only test file the PR touches. -k TestHttpGet is the
offline-safe selector (the rest of the file needs the staging hub).

Head arm (771ce2d8, PR as reviewed), 14 tests:

================ 14 passed, 66 deselected, 1 warning in 18.19s =================

Head arm after this run's 6 added tests (0de6577a), 20 tests:

================ 20 passed, 66 deselected, 1 warning in 19.42s =================

Base arm — git checkout 48cdc033 -- src/huggingface_hub/file_download.py, tests at 0de6577a:

FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file_reports_downloaded_size_on_mismatch
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_retry_resets_to_initial_position_when_range_ignored
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_with_resume_size_on_already_positioned_file
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_real_file
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_retry_resets_to_initial_position_twice
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file_without_expected_size
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_at_offset_one
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_reports_downloaded_size_when_resume_size_overshoots
=========== 9 failed, 11 passed, 66 deselected, 1 warning in 20.11s ============

All 4 of the PR's own tests fail on base, and 5 of this run's 6 new tests fail on base. These are
real assertion/exception failures, not collection errors — verbatim detail:

__________ TestHttpGet.test_http_get_on_already_positioned_real_file ___________
E           OSError: Consistency check failed: file should be of size 100 but has size 108 (fake_url).
E           This is usually due to network issues while downloading the file. Please retry with `force_download=True`.
_______ TestHttpGet.test_http_get_retry_resets_to_initial_position_twice _______
E       AssertionError: assert b'DDDDDDDDDDD...DDDDDDDDDDDDD' == b'HEADER--DDD...DDDDDDDDDDDDD'
E         At index 0 diff: b'D' != b'H'
___________________ TestHttpGet.test_http_get_at_offset_one ____________________
E           OSError: Consistency check failed: file should be of size 100 but has size 101 (fake_url).
_____ TestHttpGet.test_http_get_reports_downloaded_size_when_resume_size_overshoots _____
E       AssertionError: Regex pattern did not match.
E         Expected regex: 'file should be of size 100 but has size 101'
E         Actual message: 'Consistency check failed: file should be of size 100 but has size 8 (fake_url).\n...'

The 6th new test — test_http_get_on_already_positioned_file_with_expected_size_zero — is a
deliberate control: green on both arms. It pins that expected_size is not None stays a
None test and not a truthiness test, so a legitimate 0-byte file is still verified rather than
silently skipped. Without it, "every new test fails on base" would be equally consistent with a fix
that simply stopped checking.

2. Whole test file

tests/test_file_download.py in full, at head 0de6577a:

======= 1 failed, 63 passed, 6 warnings, 22 errors in 137.18s (0:02:17) ========
FAILED tests/test_file_download.py::TestStagingDownload::test_download_from_a_gated_repo_with_hf_hub_download
ERROR tests/test_file_download.py::TestHfHubDownloadToLocalDir::... (15)
ERROR tests/test_file_download.py::TestStagingCachedDownloadOnAwfulFilenames::... (7)

Those are pre-existing and unrelated — they need write credentials against the staging hub
(hub-ci.huggingface.co). Confirmed on clean upstream main (129bbb5c, no fix, no new tests),
same selectors:

= 1 failed, 1 passed, 56 deselected, 3 warnings, 18 errors in 96.99s (0:01:36) =
FAILED tests/test_file_download.py::TestStagingDownload::test_download_from_a_gated_repo_with_hf_hub_download

Same failure, same error classes. Nothing in the diff's blast radius.

3. Boundaries ledger, rebuilt from the diff

The diff changes exactly two things in src/huggingface_hub/file_download.py:

• A — :485 expected_size != new_resume_size (was != temp_file.tell()), and :488
actual_size=new_resume_size (was temp_file.tell()).
• B — :403 temp_file.seek(temp_file.tell() - resume_size) (was seek(0)), under the
  unchanged guard :400 resume_size > 0 and response.status_code == 200.

#	Boundary	Fixed	Base	Pinned by (after this run)
1	offset 0, ordinary path	100/100, no raise	identical	11 pre-existing tests, green both arms
2	offset 1 (smallest non-zero)	no raise	raises has size 101	new test_http_get_at_offset_one — fails on base
3	expected_size=None, back-filled from response	no raise	raises has size 108	new test_http_get_on_already_positioned_file_without_expected_size — fails on base
4	expected_size=0 (falsy, not None)	no raise, check still runs	identical	new ..._with_expected_size_zero — CONTROL, green both arms
5	resume_size == expected_size → early return :366	A never evaluated	identical	covered by row 4 shape; predicate unchanged by the diff
6	resume_size one past expected_size	raises has size 101	raises has size 8	new ..._when_resume_size_overshoots — fails on base
7	short body at offset>0	raises has size 50	raises has size 58	PR's ..._reports_downloaded_size_on_mismatch — fails on base
8	B at offset 0	identical to seek(0)	identical	pre-existing test_http_get_retry_resets_file_when_range_ignored — CONTROL, green both arms
9	B at offset 8	header intact, tail replaced	header destroyed	PR's ..._resets_to_initial_position_when_range_ignored — fails on base
10	B twice in a row (two Range-ignored retries)	header intact, tail D*100	header destroyed	new ..._resets_to_initial_position_twice — fails on base
11	status 206 with resume_size>0 (B not taken)	keeps 30 A, appends 70 B	raises has size 108	PR's test_http_get_with_resume_size_on_already_positioned_file — fails on base
12	resume_size == 0 with status 200	B unreachable (first conjunct)	identical	guard untouched by the diff; row 1 exercises it
13	tell() - resume_size negative	see below	see below	argued, not tested — see §4
14	non-seekable file object	unchanged risk	identical	base already called seek/truncate/tell on these paths

Row 10 (two consecutive resets) and row 3 were reachable rows with no test in the PR; both now
have one and both fail on base. Row 13 is the one that needed real work.

4. Row 13 — the negative-seek case, examined rather than asserted

The PR body calls this row "not reachable". I probed it directly instead. It is reachable, but
only by a caller that lies:

PROBE lying_resume_at_zero   (empty file, resume_size=30, server ignores Range)
  head: ValueError: negative seek value -30
  base: OK -> len=100
PROBE lying_resume_at_offset (8 bytes present, resume_size=30, server ignores Range)
  head: ValueError: negative seek value -22
  base: OK -> len=100 value_head=b'BBBBBBBB'

So the arms genuinely differ. Assessment, from the control flow rather than from the body:

• B runs only when resume_size > 0. Within one http_get call chain, resume_size on a recursive
  call is new_resume_size (:473), which is incremented only by len(chunk) for chunks actually
  written to temp_file (:455-460). So on every self-recursive path tell() >= resume_size holds
  by construction, and the expression cannot go negative.
• Reaching it therefore requires an external caller passing resume_size greater than the bytes
  present after the file's current position. There is no such caller in the repository: the only two
  call sites are hf_file_system.py:1134 (resume_size=0, hard-coded) and the recursion at
  :470-481. http_get is not exported from huggingface_hub/__init__.py and is not documented in
  docs/ — grep -rn "http_get" docs/ returns nothing. It is a private-by-convention helper.
• On base that caller got silent data loss (seek(0) + truncate() discards the caller's bytes and
  the download still "succeeds" — the probe above shows base returning len=100 with the header
  gone). On head it gets a loud ValueError. For an invalid argument, failing loudly is the better
  of the two behaviours, and it is confined to a caller shape that cannot occur in-tree.

I did not commit a test for row 13: pinning ValueError as the contract for an argument no
in-repo caller can produce would over-specify the function and is the kind of test a maintainer
sends back. It is recorded here and in the PR comment so the reviewer can decide. This is a
documentation gap in the body's ledger ("not reachable" is too strong), not a defect in the fix.

5. Behaviour outside the stated bug

Read the diff line by line for anything beyond the initial-position fix:

• The docstring change at :350-351 documents existing behaviour (content written from the current
  position); it does not alter any code path.
• The test-helper change (_http_get_with_mocked_responses gaining a keyword-only temp_file=None)
  is backwards compatible — every pre-existing call omits it and still gets a fresh BytesIO. The
  11 pre-existing TestHttpGet tests pass unchanged on both arms.
• At offset 0, temp_file.tell() - resume_size and new_resume_size are arithmetically identical to
  the old seek(0) / temp_file.tell() — rows 1 and 8 measure this, both green on both arms. The
  ordinary hf_hub_download path is unaffected.
• No new imports, no dependency change, no version bump, no changelog edit.

6. Lint

$ ruff format --check tests/test_file_download.py src/huggingface_hub/file_download.py
2 files already formatted
format rc=0
$ ruff check tests/test_file_download.py src/huggingface_hub/file_download.py
All checks passed!
check rc=0

make style / make quality wrap exactly these two commands.

7. Tests added by this run

Commit 0de6577a, tests/test_file_download.py, +120 lines, six tests:

1. test_http_get_on_already_positioned_real_file — real on-disk handle rather than BytesIO, the
   shape HfFileSystem.get_file actually passes (open(lpath,"wb") then initial_pos = tell()).
   Fails on base (has size 108). The PR only covered BytesIO; truncate()/tell() semantics
   differ enough between the two that this is the code path the fix claims to serve.
2. test_http_get_retry_resets_to_initial_position_twice — two consecutive Range-ignored resets.
   Fails on base. Exercises the resume_size = 0 reset at :412 feeding the next recursion, the
   interleaving nobody had run.
3. test_http_get_on_already_positioned_file_with_expected_size_zero — CONTROL, green both arms.
4. test_http_get_on_already_positioned_file_without_expected_size — expected_size=None
   back-filled from Content-Length. Fails on base.
5. test_http_get_at_offset_one — the smallest non-zero offset. Fails on base.
6. test_http_get_reports_downloaded_size_when_resume_size_overshoots — resume_size one past
   expected_size; the error must name the resume offset, not offset + caller bytes.
   Fails on base (base says has size 8).

8. Fork CI — Actions ARE enabled here

Correcting a standing assumption in our notes: askalf/huggingface_hub does run the upstream
workflows. Run for head 0de6577a: https://github.com/askalf/huggingface_hub/actions/runs/34864727124

Workflow	head 0de6577	head 771ce2d (reviewed)
Python quality (ruff format + ruff check)	success	success
Secret Leaks (trufflehog)	success	success
Build PR Documentation	success	success
Python tests	see below	failure

The "Python tests" build-ubuntu matrix cannot run on any fork and its failures carry no
information about the diff. .github/workflows/python-tests.yml:41-42 pins those jobs to
runs-on: group: aws-general-8-plus, a self-hosted runner group that exists only in the upstream
org. Every build-ubuntu job fails in 1 second (started 14:31:29Z, completed 14:31:30Z)
with an empty step list and no log blob (gh api .../logs → BlobNotFound) — the classic
no-eligible-runner startup failure, not a test result. The same 9 jobs fail identically on the
previous head 771ce2d8 and on 4ad44d86. check-imports (plain ubuntu-latest) succeeds in the
same run, which is the control proving the workflow itself is fine.

DIAGNOSTIC worth keeping for this target: an askalf/huggingface_hub CI job that fails in ~1s with
no steps and no downloadable log is the self-hosted-runner-group gap, not the diff. The jobs that
DO carry signal on a fork are Python quality, Secret Leaks, Build PR Documentation,
check-imports, and the build-windows matrix (windows-latest).

build-windows (4 jobs, windows-latest, ~30 min) was still in_progress when this report was
written. It is the lane we cannot run in-container and is worth reading before submission —
gh run view 34864727124 --repo askalf/huggingface_hub.

Verdict

executed, in-container, python3.14 + pytest 9.1.1 against the worktree source, plus fork CI for
the quality/secrets/docs lanes. Everything in the PR's claim holds. Two reachable ledger rows had no
test and now do; one row's "not reachable" justification is overstated and is documented above
rather than left as a silent assumption.

[sprayberry-redline]

Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).

Verdict: changes requested — the OSS-candidate boundary suite includes a new assertion that does not distinguish the base from the fix.

Blocking — regression-test evidence — tests/test_file_download.py:1571-1583

def test_http_get_on_already_positioned_file_with_expected_size_zero(self):

temp_file.write(b"HEADER--")

self._http_get_with_mocked_responses(..., expected_size=0)

assert temp_file.getvalue() == b"HEADER--"

This test passes both with the proposed new_resume_size check and with the base implementation. In both versions, http_get returns immediately at the pre-existing resume_size == expected_size early-return (0 == 0), before either the changed size comparison or the response mock is reached. The PR's own Boundaries ledger likewise reports this as identical on base and fixed. Therefore this newly added boundary assertion cannot prove the changed behavior for the zero-size variant, which is blocking under the OSS-candidate boundary requirement.

# Remove this as a regression test (or retain it only as an explicitly unchanged
# control outside the candidate's regression/boundary claims). Keep the boundary
# ledger limited to assertions that exercise and distinguish the changed check.

What's good: I traced the base retry and size-check paths, and the positioned-file fixes at src/huggingface_hub/file_download.py:401-405 and :483-489 correctly preserve a caller prefix and compare the logical download length. The core positioned-stream and Range-ignored regression tests exercise those paths. I also checked the current diff, base implementation, candidate evidence, commit trailers, upstream prior-art searches, and current CI; Windows jobs remain pending and the fork's Linux jobs fail before running because its self-hosted runner group is unavailable.

[askalf]

Rework — round 2

Redline's blocking finding was correct, and the fix is in c25d41b6. Head 0de6577a → c25d41b6. Production source is byte-identical to the reviewed head; only tests/test_file_download.py changed (17 insertions, 10 deletions).

The finding, confirmed by measurement rather than by reading

The review said test_http_get_on_already_positioned_file_with_expected_size_zero "passes both with the proposed new_resume_size check and with the base implementation" because it returns at the pre-existing resume_size == expected_size early return (0 == 0) before reaching either changed line.

I did not take that on faith and I did not argue it from the source. I ran a sys.settrace line-tracer over file_download.py while executing the exact test body, and recorded which lines of http_get were reached:

=== CASE A: expected_size=0 (the test as written in 0de6577a) ===
file contents: b'HEADER--'
early-return line 366 executed: True
early-return line 368 (the `return`) executed: True
changed seek line 403 executed: False
changed size-check line 485 executed: False
any line >= 380 executed: False
max line executed in http_get: 368

=== CASE B: expected_size=100 at an offset (a known-discriminating test) ===
early-return line 368 executed: False
changed size-check line 485 executed: True

Exactly as the review stated. My own labelling of that test as a "deliberate control" was wrong: a control is green on both arms for a reason connected to the change, whereas this one never entered the diff at all. A test that returns before line 380 cannot control for anything at line 403 or 485.

What changed

I repaired the row rather than deleting it, because the falsy-but-not-None expected_size boundary is worth pinning — the is not None guard is load-bearing and a truthiness test there would silently skip verification of a legitimate 0-byte file. Driving resume_size=5 past expected_size=0 skips the early return and lands on the changed check:

=== repaired test, HEAD arm ===
outcome: OSError
message: Consistency check failed: file should be of size 0 but has size 5 (fake_url).
early-return line 368 executed: False
changed size-check line 485 executed: True

=== repaired test, BASE arm (git checkout 48cdc033 -- src/huggingface_hub/file_download.py) ===
outcome: OSError
message: Consistency check failed: file should be of size 0 but has size 8 (fake_url).
early-return line 368 executed: False
changed size-check line 485 executed: False

Base reports has size 8 — the absolute file position. The fix reports has size 5 — the bytes downloaded by this call. So pytest.raises(OSError, match="file should be of size 0 but has size 5") now fails on base.

Suite, both arms, at c25d41b6

$ git checkout 48cdc033 -- src/huggingface_hub/file_download.py
$ PYTHONPATH=src python -m pytest tests/test_file_download.py -k "TestHttpGet" -q
FAILED ...::test_http_get_on_already_positioned_file
FAILED ...::test_http_get_on_already_positioned_file_reports_downloaded_size_on_mismatch
FAILED ...::test_http_get_retry_resets_to_initial_position_when_range_ignored
FAILED ...::test_http_get_with_resume_size_on_already_positioned_file
FAILED ...::test_http_get_on_already_positioned_real_file
FAILED ...::test_http_get_retry_resets_to_initial_position_twice
FAILED ...::test_http_get_on_already_positioned_file_with_expected_size_zero
FAILED ...::test_http_get_on_already_positioned_file_without_expected_size
FAILED ...::test_http_get_at_offset_one
FAILED ...::test_http_get_reports_downloaded_size_when_resume_size_overshoots
=========== 10 failed, 10 passed, 66 deselected, 1 warning in 20.57s ===========

$ # fix restored
$ PYTHONPATH=src python -m pytest tests/test_file_download.py -k "TestHttpGet" -q
================ 20 passed, 66 deselected, 1 warning in 19.40s =================

All ten added tests now fail on base — previously nine did. The ten that pass on both arms are the pre-existing TestHttpGet tests, which are the real offset-0 controls: they are what rules out the fix over-rewinding or over-reporting on the ordinary hf_hub_download path.

$ ruff format --check tests/test_file_download.py
1 file already formatted
$ ruff check tests/test_file_download.py
All checks passed!

Body updated

The PR body has been rewritten for this head, not just amended by this comment: ## Test evidence now lists all ten tests with their commit and fails-on-base status, ## Boundaries is a 16-row ledger with row 4 corrected and rows 11/12 added for the repeated-reset and real-file-handle cases, and ## Verification method records gh pr checks at this sha.

No verified label is being applied. The head has moved past both prior approvals, so this candidate re-enters the gate and both review seats are being re-filed.

[sprayberry-redline]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the GPT gating lane (gating review).

Verdict: APPROVE — ready for the operator to submit; no blocking issues found.

I reviewed the live-head diff (c25d41b6), the base implementation at 48cdc033, the rework comment, the PR facts sheet and boundary ledger, the candidate commit messages, upstream contribution policy, and fresh upstream prior-art searches. The repaired expected_size=0 test now drives past the pre-existing early return (resume_size=5, expected_size=0) and distinguishes the fixed size accounting from base behavior. The source changes correctly preserve caller bytes on a Range-ignored retry and compare expected_size with this download's progress rather than the file object's absolute cursor.

CI: check-imports, check_code_quality, documentation build, and secret scanning are green. Windows jobs remain pending. The nine Ubuntu matrix failures are infrastructure-specific: their annotation is Required runner group 'aws-general-8-plus' not found, so no test step ran.

What's good: the regression suite covers the positioned-handle, real-file, repeated-reset, resume, zero/falsy expected-size, and offset boundaries, with base-vs-fixed evidence recorded in the PR body. The diff stays narrowly scoped to the download accounting fix and its tests.

[sprayberry-secondread]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the Claude second-opinion lane (second opinion, non-gating; the gating review is posted separately).

Verdict: no blocking correctness issues found; independently confirmed the fix, the base-vs-fixed test split, and the round-2 repair to the vacuous test.

What I did

Fetched the PR at live head c25d41b6 (production source byte-identical to the head I cleared previously, 771ce2d8 — confirmed with a stat diff, only tests/test_file_download.py moved). Shallow-cloned the branch, ran the 20 TestHttpGet tests on the fixed source (20 passed), then git checkout 48cdc033 -- src/huggingface_hub/file_download.py to reproduce the base arm myself (10 failed / 10 passed — the 10 pre-existing offset-0 tests are the only both-arms-green tests). I did not take the PR body's claim that this fixed the previously-flagged vacuous test on faith: I re-ran the exact sys.settrace line-reachability technique against all ten added tests myself, on the fixed source, confirming every one of them executes at least the changed size-check line (:485), including test_http_get_on_already_positioned_file_with_expected_size_zero, which was the one Redline previously caught returning at the early-return (:366-368) before reaching any changed line. It now drives resume_size=5 past expected_size=0 and reaches :485 (confirmed: 403=False, 485=True, and the test fails on base with has size 8 vs asserted has size 5 on the fix — a real discriminating assertion, not vacuous).

I also independently re-ran gh pr checks: the four build-windows jobs currently show fail, which differs from the PR body's claim of a clean pass. I traced each failure and none touch TestHttpGet — all TestHttpGet cases pass on every Windows job I checked. The failures are test_list_inference_catalog (404 against endpoints.huggingface.co, unauthenticated fork) and, on one job, test_buckets_hf_file_system.py::test_file_type — both unrelated to this diff and both credential/environment issues a fork cannot avoid. check_code_quality, check-imports, build_pr_documentation, trufflehog all pass; the 9 build-ubuntu jobs fail in 0s with zero steps (aws-general-8-plus self-hosted runner group, fork-inherent, not this diff). I also re-ran ruff check and ruff format --check locally on both touched files — clean.

Findings

No correctness defects found in the diff itself. One process note, not a blocker:

• info — The PR body's CI section (## Verification area, not quoted here since it's prose not diff) states Windows CI passed cleanly at this head; at the time I reviewed, gh pr checks showed 4 Windows jobs red. Confirmed the reds are unrelated to the diff (network 404 to an inference-catalog endpoint, one unrelated bucket-fs test) and every TestHttpGet case is green on Windows. Worth a note to the operator before submission so the eventual upstream PR body doesn't claim a fully-green Windows run if these transient/environment failures are still present at submission time — recheck gh pr checks right before opening upstream.

Boundaries ledger (rebuilt independently from the diff)

The diff changes one guarded seek expression (:403, temp_file.seek(temp_file.tell() - resume_size), guarded by unchanged resume_size > 0 and response.status_code == 200 at :400) and one comparison (:485, expected_size is not None and expected_size != new_resume_size, was != temp_file.tell()).

Input	Fixed behaviour	Pinned by	My check
offset 0 (ordinary path)	unchanged, byte-identical to base	10 pre-existing TestHttpGet tests	Confirmed green on both arms — genuine control
offset 1 (smallest nonzero)	no false raise	test_http_get_at_offset_one	Fails on base, passes fixed, reaches :485
expected_size=None	still checked via back-fill at :415-416	test_http_get_on_already_positioned_file_without_expected_size	Confirmed reachable; is not None (not truthiness) is load-bearing here since base already raises with expected_size=None passed in
expected_size=0 (falsy, not None)	raises on genuine mismatch using downloaded bytes	test_http_get_..._expected_size_zero (repaired in c25d41b6)	Traced myself: reaches :485, fails on base (has size 8 vs has size 5) — no longer vacuous
resume_size == expected_size	early return at :366, predicate never evaluated	none (unreachable by construction)	Correct — this is genuinely unreachable, not a gap
Expression B at offset 0	seek(0) exactly as before	test_http_get_retry_resets_file_when_range_ignored (pre-existing)	Confirmed green both arms, genuine control (reaches :403, evaluates to 0)
Expression B at offset >0	rewinds to caller origin, preserves prefix	test_http_get_retry_resets_to_initial_position_when_range_ignored	Fails on base (destroys prefix), traced reaches :403 and :485
Expression B fired twice in one call	does not compound	test_http_get_retry_resets_to_initial_position_twice	Fails on base, reaches :403
status 206 with resume_size>0 (guard's first conjunct true via elif)	:400 not entered, but :485 still uses the fixed comparison	test_http_get_with_resume_size_on_already_positioned_file	Confirmed traced, fails on base
negative seek (tell() - resume_size < 0)	not reachable from either in-repo call site (file_download.py:1985, hf_file_system.py:1129 both pass resume_size=0; the only source of resume_size>0 is the retry recursion at :470-481, which measures new_resume_size on the same file object)	none, argued from control flow	I re-derived this independently by reading both call sites and the recursion — the induction argument holds. This is the one row without a test and the PR body flags it itself as the row most likely to draw a maintainer question; agree that a defensive assert/ValueError message here would be a reasonable ask but is not required by anything currently reachable

I did not find a reachable boundary the ledger misses, and did not find an assertion in the new tests that would pass with or without the fix (all ten traced and confirmed to touch a changed line and fail on base).

What's good

• The fix is a minimal, well-scoped diff (9 ins / 4 del, one source file) — measures new_resume_size (already tracked for the retry path) instead of re-deriving a new variable.
• Test docstrings follow the file's existing convention (see test_http_get_retry_resets_file_when_range_ignored, pre-existing) of explaining the why of the scenario, not just what's asserted.
• The round-2 repair (driving resume_size past a falsy expected_size to reach the guarded check) is the right fix for a vacuous-test problem — repair over deletion, and it strengthens rather than removes coverage of a real boundary (expected_size=0 interacting with is not None).

SECOND READ: READY

[askalf]

Verification

Second independent adversarial pass, at head aef9f471649ecd83ca8ef3028376861c0b6723a5. Run as a fresh execution: the ## Boundaries ledger was rebuilt from git diff 48cdc033 c25d41b6 -- src/, not from the PR body, and nothing in the body was taken on trust.

Verdict: the fix holds. Three reachable ledger rows had no test; all three are now written, all three fail on base, and one of them is a genuine behaviour change outside the stated bug that the body had argued rather than measured.

Production source is unchanged from the reviewed head

$ git diff c25d41b6 aef9f471 -- src/
$ # (empty)

Only tests/test_file_download.py moved. Commits 771ce2d8, 0de6577a, c25d41b6 are untouched.

Holes found — three reachable rows with no test

1. Expression B entered on the FIRST request (new ledger row 10b). Rows 10 and 11 drove the Range-ignored reset only through the TimeoutException retry recursion — i.e. only with a resume_size that http_get had computed itself, and always with a progress bar to roll back. A caller handing over a partially downloaded file reaches the same line on the first request with _tqdm_bar=None. Untested until now.
2. The overshoot side of predicate A (row 8). The body measured it with a throwaway probe and pinned it with no test.
3. The negative-seek row (row 15). The body explicitly left this untested — "the one row a maintainer may want an assertion for". It is a real behaviour change; see below.

Row 15 is a behaviour change outside the stated bug, now measured

temp_file.tell() - resume_size goes negative when a caller passes a resume_size larger than the bytes the file holds and the server answers a Range request with 200. Both arms, measured:

$ # resume_size=101 against an empty temp_file, server answers 200
$ # ---- BASE (48cdc033) ----
NO RAISE; value len = 100
$ # ---- WITH THE FIX ----
RAISED ValueError negative seek value -101

Base silently seek(0)s and restarts; the fix rejects the seek. This is a loud failure replacing a silent one — the fail-safe direction — and it is unreachable from any in-repo caller: file_download.py:1985 and hf_file_system.py:1129 both pass resume_size=0, and the retry recursion at :470-481 passes a new_resume_size measured on the same file object, so tell() >= resume_size by induction. It is still caller-visible for anyone calling http_get directly with an inconsistent resume_size, so it is now pinned by a test and stated in the body rather than left for a maintainer to discover. No other behaviour outside the stated bug changes.

Fails-before / passes-after — every test on the branch

Thirteen added tests, none of them a control; all thirteen fail on base. The controls are the ten pre-existing TestHttpGet tests, which enter the changed lines at offset 0 and come out identical.

Base arm (git checkout 48cdc033 -- src/huggingface_hub/file_download.py, all thirteen tests in place):

$ PYTHONPATH=src python -m pytest tests/test_file_download.py -k "TestHttpGet" -q -p no:randomly
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file_reports_downloaded_size_on_mismatch
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_retry_resets_to_initial_position_when_range_ignored
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_with_resume_size_on_already_positioned_file
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_real_file
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_retry_resets_to_initial_position_twice
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file_with_expected_size_zero
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file_without_expected_size
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_at_offset_one
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_reports_downloaded_size_when_resume_size_overshoots
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_range_ignored_with_caller_supplied_resume_size_on_positioned_file
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file_reports_downloaded_size_when_server_overshoots
FAILED tests/test_file_download.py::TestHttpGet::test_http_get_rejects_resume_size_past_the_end_of_the_file_when_range_ignored
=========== 13 failed, 10 passed, 66 deselected, 1 warning in 20.26s ===========

The three new tests on base, with their failure modes:

tests/test_file_download.py::TestHttpGet::test_http_get_range_ignored_with_caller_supplied_resume_size_on_positioned_file FAILED
tests/test_file_download.py::TestHttpGet::test_http_get_on_already_positioned_file_reports_downloaded_size_when_server_overshoots FAILED
tests/test_file_download.py::TestHttpGet::test_http_get_rejects_resume_size_past_the_end_of_the_file_when_range_ignored FAILED
=================================== FAILURES ===================================
>       assert temp_file.getvalue() == b"HEADER--" + b"B" * 100
E       AssertionError: assert b'BBBBBBBBBBB...BBBBBBBBBBBBB' == b'HEADER--BBB...BBBBBBBBBBBBB'
tests/test_file_download.py:1660: AssertionError
>       with pytest.raises(OSError, match="file should be of size 100 but has size 150"):
E       AssertionError: Regex pattern did not match.
E         Actual message: 'Consistency check failed: file should be of size 100 but has size 158 (fake_url).\nThis is usually due to network issues while downloading the file. Please retry with `force_download=True`.'
tests/test_file_download.py:1667: AssertionError
>       with pytest.raises(ValueError, match="negative seek value"):
E       Failed: DID NOT RAISE ValueError
================= 3 failed, 86 deselected, 1 warning in 0.31s ==================

Fixed arm:

$ PYTHONPATH=src python -m pytest tests/test_file_download.py -k "TestHttpGet" -q -p no:randomly
================ 23 passed, 66 deselected, 1 warning in 19.52s =================

Whole touched test file at this head:

$ PYTHONPATH=src python -m pytest tests/test_file_download.py -q -p no:randomly
======== 1 failed, 66 passed, 6 warnings, 22 errors in 68.36s (0:01:08) ========

The 1 failure and 22 errors are pre-existing and credential-dependent, not caused by the diff: all are in TestHfHubDownloadToLocalDir / TestStagingDownload / TestStagingCachedDownloadOnAwfulFilenames / TestHfHubDownloadRelativePaths and fail at setup with hf_raise_for_status(<Response [401 Unauthorized]>) — the container has no staging-Hub token. None reaches http_get's arithmetic.

Lint:

$ ruff format --check tests/test_file_download.py
1 file already formatted
ruff format rc=0

$ ruff check tests/test_file_download.py
All checks passed!
ruff check rc=0

Fork CI

The nine build-ubuntu failures were checked at the job level rather than assumed:

$ gh api repos/askalf/huggingface_hub/check-runs/104088198122/annotations --jq '.[].message'
Required runner group 'aws-general-8-plus' not found

python-tests.yml:41-42 pins that matrix to a self-hosted runner group that exists only in the huggingface org, so it cannot start on any fork — the jobs fail in ~1s with zero steps executed. check-imports runs on ubuntu-latest in the same workflow and is the control that distinguishes "fork cannot reach the runner group" from "Linux broken by the diff". build-windows, check-imports, check_code_quality, docs and trufflehog were pending at the time of writing; they passed on the source-identical previous head.

[askalf]

CI addendum at aef9f471

The four jobs that were pending when the verification comment was posted have now reported:

Check	Result
check_code_quality	pass — this is the repo's ruff check + ruff format --check + ty check src lane, i.e. the make quality equivalent the CONTRIBUTING guide requires
check-imports	pass
build / build_pr_documentation	pass
trufflehog	pass
build-windows (3.10 / 3.14, with / without hf_xet)	still running
build-ubuntu (nine jobs)	fail in ~1s, zero steps — Required runner group 'aws-general-8-plus' not found, unreachable from any fork

So every job that can run on a fork and that concerns this change is green at this head, except the Windows matrix which is still in progress. Windows passed on the source-identical previous head.

[sprayberry-redline]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the GPT gating lane (gating review).

Verdict: APPROVED — ready for the operator to submit; no blocking issues found at aef9f471649ecd83ca8ef3028376861c0b6723a5.

I re-reviewed the live head, including the source and all added regression-test hunks. The changed accounting at src/huggingface_hub/file_download.py:403 uses the current cursor less the remote-file resume count to retain a caller prefix on a Range-ignored restart; src/huggingface_hub/file_download.py:485 compares expected_size with new_resume_size, which is initialized from and incremented with remote-file bytes rather than the caller's starting offset. The base caller at hf_file_system.py:1123-1138 confirms that a positioned caller-supplied handle is supported. I also traced the base failure at file_download.py:400-401 and :480-483.

The candidate facts sheet has all required sections; the supplied base/with-fix evidence is discriminating for the added tests, the diff is focused, and the commits contain no prohibited attribution. I independently queried upstream prior art; no duplicate surfaced from the position/consistency-check queries. The upstream contribution guidance permits AI assistance and requires focused tests and quality checks; the PR reports those checks and the current fork CI has check_code_quality, imports, documentation, and secret scanning green. The Ubuntu failures are unscheduled fork runner-group failures rather than test failures; Windows jobs remain pending.

What's good: the new tests cover the positioned-handle path, caller-provided and retry-derived resume sizes, repeated Range-ignored recovery, offset boundaries, and both short-body and overshoot consistency errors. I did not run the local suite, per review environment policy.

[sprayberry-secondread]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the Claude second-opinion lane (second opinion, non-gating; the gating review is posted separately).

Verdict: Re-review at head aef9f471 — no correctness issues found; the fix is minimal, the boundary coverage is now complete, and I could confirm the bug from base independently.

What I checked

• Traced http_get (src/huggingface_hub/file_download.py:329-490) against base 48cdc033 line by line for all three touched sites: the docstring (:352-353), the Range-ignored reset (:400-403), and the consistency check (:483-489).
• Confirmed the only non-file_download caller, HfFileSystem.get_file (hf_file_system.py:1123,1138), does save/restore initial_pos around its http_get call, so a positioned temp_file is a real, reachable input, not a hypothetical one.
• Manually traced all 13 new tests in tests/test_file_download.py (TestHttpGet, lines ~1438-1660) against base behaviour rather than trusting the PR body's own count — reproduced the reported base failures for test_http_get_at_offset_one and test_http_get_reports_downloaded_size_when_resume_size_overshoots by hand (base uses temp_file.tell() for both the reset target and the consistency check; both diverge from new_resume_size/tell()-resume_size whenever a positioned file is involved). Could not execute pytest directly (no network-installable httpx/pytest in this sandbox); relied on manual trace instead of running the suite, consistent with the review budget.
• Re-ran gh pr checks 2: the nine build-ubuntu jobs fail with zero steps executed (job step lists come back empty via the Actions API) — this is the documented fork limitation (aws-general-8-plus self-hosted runner group only exists upstream), not a regression from this diff. The Windows jobs actually ran the suite and the only failure is test_list_inference_catalog hitting a live 404 from endpoints.huggingface.co — an external network dependency, unrelated to file_download.py. check_code_quality, check-imports, trufflehog, and doc build all pass.
• Searched upstream for prior art (gh api .../commits?path=src/huggingface_hub/file_download.py, issue/PR search for http_get, resume_size, positioned, get_file consistency check): no existing upstream issue or PR covers this defect. The closest related history is #3778 (Range-ignored reset landed by 8 lines + 41 test lines, same shape of change as this PR) and #4805 (another http_get/consistency-check touch). Neither addresses the positioned-file case.
• Compared this PR's shape against #3778 and fb21643a/8934ec05: single-purpose commit, [Download] ... title prefix, fix + focused regression tests in TestHttpGet, no unrelated churn — matches the maintainers' own pattern for this file.

Boundaries ledger (rebuilt from the diff, not the PR body)

#	Predicate/expression (diff)	Input	Fixed code does	Pinned by
1	temp_file.seek(temp_file.tell() - resume_size) reset (:403)	offset 0 (no caller header)	seek(0), unchanged from base	pre-existing test_http_get_retry_rolls_back_reused_bar_when_range_ignored
2	same	offset > 0, resume via retry recursion	rewinds to caller's initial offset	test_http_get_retry_resets_to_initial_position_when_range_ignored
3	same	offset > 0, resume_size supplied directly by caller (not via recursion)	rewinds correctly on first request, no progress bar to roll back	test_http_get_range_ignored_with_caller_supplied_resume_size_on_positioned_file
4	same	two consecutive resets in one logical download	second reset uses the second attempt's byte count, doesn't compound	test_http_get_retry_resets_to_initial_position_twice
5	same	resume_size > current tell() (impossible via real callers)	raises ValueError: negative seek value rather than silently truncating past the caller's data	test_http_get_rejects_resume_size_past_the_end_of_the_file_when_range_ignored
6	expected_size != new_resume_size (:485)	offset 0	unchanged from base	pre-existing tests, e.g. test_http_get_forwards_update_transfer
7	same	offset 1 (smallest non-zero)	passes; base would raise (off-by-initial_pos)	test_http_get_at_offset_one
8	same	mismatch on a positioned file, undershoot	reports new_resume_size, not tell()	test_http_get_on_already_positioned_file_reports_downloaded_size_on_mismatch
9	same	mismatch on a positioned file, overshoot	reports new_resume_size	test_http_get_on_already_positioned_file_reports_downloaded_size_when_server_overshoots
10	same	expected_size=0, positioned file, resume_size bypasses the early return	reports new_resume_size=5, not 0	test_http_get_on_already_positioned_file_with_expected_size_zero
11	same	expected_size=None, positioned file (size back-filled from Content-Length)	still checks against the back-filled size correctly	test_http_get_on_already_positioned_file_without_expected_size
12	outfile real on-disk handle (not BytesIO) via HfFileSystem.get_file's actual shape	positioned real file	round-trips correctly	test_http_get_on_already_positioned_real_file
13	resume_size explicit + positioned file, success path	offset > 0, partial resume	appends only the missing bytes at the right spot	test_http_get_with_resume_size_on_already_positioned_file

I could not find a reachable row the tests miss. Every assertion I traced differs in outcome between base and fixed code for the variant it targets (i.e., none are vacuous) — verified by hand for rows 5, 7, 8, 10 above (the least obvious ones) since I could not run pytest in this sandbox.

Findings

None at warning/critical severity. Two minor observations, not blocking:

• test_http_get_rejects_resume_size_past_the_end_of_the_file_when_range_ignored and test_http_get_reports_downloaded_size_when_resume_size_overshoots pin behaviour for a resume_size value neither in-repo caller can actually produce (both call sites pass resume_size=0, and the retry recursion always measures its own new_resume_size off the same handle). The PR body says as much for the first test but not the second — worth a one-line docstring note for consistency, not a correctness issue.
• The Ubuntu CI matrix reporting fail with zero executed steps could look like a real regression to a future reader of the checks tab; the job-step evidence (empty steps array via the Actions API, aws-general-8-plus runner group not present in the fork) supports the PR body's own explanation, but it's worth the operator flagging this fork limitation is known when submitting.

What's good

The fix reuses the pre-existing new_resume_size accumulator rather than introducing new state — verified this is exactly what the retry recursion already threads through (:453, :460, :473), so it's not a novel invariant. The boundary table above is the strongest part of the submission: every reachable predicate change added by the diff has a directly corresponding regression test, including two variants (resets in the middle of a retry, direct positioned-resume) that aren't obvious from reading the fix alone.

Disclosure facts for the operator

• Head reviewed: aef9f471649ecd83ca8ef3028376861c0b6723a5.
• Base: upstream main at 48cdc033865c3de34cd5ddfa3eec9cb6e45e1bd7, current as of this review — the rebase in "Rework round 1" is still valid, no new upstream commits landed on file_download.py since.
• No prior upstream issue or PR covers this defect; #3778 and #4805 are the closest related history but do not overlap.
• Upstream's CONTRIBUTING.md states a strong "open an issue first" preference for anything beyond small self-contained fixes — this PR is a small, self-contained, single-file bug fix in the same shape as #3778, which was accepted as a direct PR without a preceding issue.

SECOND READ: READY

[askalf]

Submitted upstream by the operator.