From 2e573c60049c1734b5f02e9878df8946b216a9a2 Mon Sep 17 00:00:00 2001 From: Divyam Talwar Date: Sun, 20 Sep 2026 02:47:08 +0530 Subject: [PATCH] fix(fetch): create destination parents for nested downloads Address #9 with focused regression coverage. AI-assisted implementation and isolated source review; exact validation and remaining platform limitations are recorded in the draft PR. Signed-off-by: Divyam Talwar --- QUICKSTART.md | 4 + quantprobe/fetch.py | 5 + tests/smoke.py | 7 ++ tests/test_fetch_destinations.py | 156 +++++++++++++++++++++++++++++++ 4 files changed, 172 insertions(+) create mode 100644 tests/test_fetch_destinations.py diff --git a/QUICKSTART.md b/QUICKSTART.md index 69b20a4..1f13b40 100644 --- a/QUICKSTART.md +++ b/QUICKSTART.md @@ -166,6 +166,10 @@ quantprobe run --gguf ./models/Qwen3-30B-A3B-Q2_K.gguf quantprobe bench --gguf ./models/Qwen3-30B-A3B-Q2_K.gguf ``` +`fetch` creates a missing destination directory and any parent directories in an explicitly +requested nested filename. A regular file blocking that directory remains an error. + + ### Make your own compressed model The one-command version — picks a requantizable source from the repo, fetches the eval corpus, diff --git a/quantprobe/fetch.py b/quantprobe/fetch.py index 6d310b1..7274cad 100644 --- a/quantprobe/fetch.py +++ b/quantprobe/fetch.py @@ -60,6 +60,11 @@ def fetch(repo, dest, fname, tok, tries=100, force=False): os.remove(out) if os.path.exists(part): os.remove(part) + # The CLI's run() does not create dest, and a remote filename may add nested + # directories even when callers such as auto have already created dest. + parent = os.path.dirname(part) + if parent: + os.makedirs(parent, exist_ok=True) r = requests.head(url, headers=hdr0, allow_redirects=True, timeout=60) total = int(r.headers.get("Content-Length", 0)) print(f" {fname}: {total / 1e9:.2f} GB", flush=True) diff --git a/tests/smoke.py b/tests/smoke.py index fce0397..115a353 100644 --- a/tests/smoke.py +++ b/tests/smoke.py @@ -1782,6 +1782,13 @@ class _R: assert rc == 0 and "--force" in out +def t_fetch_creates_the_destination_it_was_handed(): + # `fetch.run` passes --dest straight through, so a missing dest (or a remote filename with + # its own subdirectory) used to kill the download on open(). Full cases in the module. + from tests.test_fetch_destinations import run_smoke + return run_smoke() + + def t_c11_depth_aware_dense_split(): # C-11 (prereg #66): the dense split must budget for the desktop reserve + compute buffer and # shrink its GPU layer count as context deepens - the old flat vc*0.9 emitted a 16k config diff --git a/tests/test_fetch_destinations.py b/tests/test_fetch_destinations.py new file mode 100644 index 0000000..5a7ccba --- /dev/null +++ b/tests/test_fetch_destinations.py @@ -0,0 +1,156 @@ +"""`fetch` must be able to write to the destination it was handed. + +`quantprobe fetch ` dispatches through ``fetch.run``, which passes +``--dest`` straight to ``fetch()`` - unlike the ``python -m quantprobe.fetch`` entry point, +which has always done ``os.makedirs(dest)`` first. So the CLI died on ``open(part, mode)`` +with FileNotFoundError whenever dest did not exist yet, and also whenever the *remote* +filename carried its own subdirectory (repos nest split shards under a quant folder), which +no dest-level mkdir would have covered either. + +Synthetic HTTP throughout: no network, no real repo, no credential read. +""" + +from __future__ import annotations + +import argparse +import contextlib +import os +import tempfile +from unittest import mock + +from quantprobe import fetch as fmod + +BODY = b"GGUF" + b"\x00" * 60 + + +class _Response: + """Only the surface `fetch` touches: status, Content-Length, chunked body.""" + + def __init__(self, body, status=200): + self._body, self.status_code = body, status + self.headers = {"Content-Length": str(len(body))} + + def iter_content(self, n): + for i in range(0, len(self._body), n): + yield self._body[i : i + n] + + +@contextlib.contextmanager +def synthetic_http(body=BODY, forbid_get=False): + """Stub requests.head/get for the duration; count the calls so a "skip" that quietly + re-downloads cannot pass.""" + calls = {"head": 0, "get": 0} + + def _head(url, **kw): + calls["head"] += 1 + return _Response(body) + + def _get(url, **kw): + calls["get"] += 1 + if forbid_get: + raise AssertionError("fetch re-downloaded a file it reported as already complete") + return _Response(body) + + with ( + mock.patch.object(fmod.requests, "head", _head), + mock.patch.object(fmod.requests, "get", _get), + ): + yield calls + + +def test_fetch_creates_a_missing_destination_directory(): + with tempfile.TemporaryDirectory() as tmp: + dest = os.path.join(tmp, "weights", "gguf") + with synthetic_http() as calls: + assert fmod.fetch("org/repo", dest, "model.gguf", None) is True + out = os.path.join(dest, "model.gguf") + with open(out, "rb") as f: + assert f.read() == BODY + assert not os.path.exists(out + ".part"), ".part must be renamed away on completion" + assert calls["get"] == 1 + + +def test_fetch_creates_the_subdirectory_named_by_the_remote_filename(): + # The filename is a remote path, not a bare basename - `UD-Q2_K_XL/...` is how the split + # shards of a published build are addressed, and it needs a directory under dest. + fname = "UD-Q2_K_XL/model-00001-of-00002.gguf" + with tempfile.TemporaryDirectory() as tmp: + dest = os.path.join(tmp, "weights") + with synthetic_http(): + assert fmod.fetch("org/repo", dest, fname, None) is True + with open(os.path.join(dest, "UD-Q2_K_XL", "model-00001-of-00002.gguf"), "rb") as f: + assert f.read() == BODY + + +def test_fetch_still_writes_into_a_destination_that_already_exists(): + with tempfile.TemporaryDirectory() as dest: + with synthetic_http(): + assert fmod.fetch("org/repo", dest, "model.gguf", None) is True + assert os.path.getsize(os.path.join(dest, "model.gguf")) == len(BODY) + + +def test_an_already_complete_output_is_still_skipped_untouched(): + # U-18's skip must survive: a matching-size file on disk is not re-downloaded and not + # rewritten, and creating parents must not become a reason to touch it. + with tempfile.TemporaryDirectory() as tmp: + dest = os.path.join(tmp, "weights") + os.makedirs(dest) + out = os.path.join(dest, "model.gguf") + with open(out, "wb") as f: + f.write(BODY) + before = os.stat(out) + with synthetic_http(forbid_get=True) as calls: + assert fmod.fetch("org/repo", dest, "model.gguf", None) is True + assert calls["get"] == 0 + after = os.stat(out) + assert (after.st_size, after.st_mtime_ns) == (before.st_size, before.st_mtime_ns) + + +def test_a_parent_path_blocked_by_a_file_still_raises(): + # A regular file sitting where a directory must go is a real error. It must stay an error - + # not be swallowed, and not clobber the file that is in the way. + with tempfile.TemporaryDirectory() as tmp: + blocker = os.path.join(tmp, "weights") + with open(blocker, "wb") as f: + f.write(b"not a directory") + with synthetic_http() as calls: + try: + fmod.fetch("org/repo", blocker, "model.gguf", None) + except OSError: + pass + else: + raise AssertionError("a file blocking the parent path must not be worked around") + assert calls == {"head": 0, "get": 0}, "blocked parents must fail before HTTP" + assert os.path.isfile(blocker) + with open(blocker, "rb") as f: + assert f.read() == b"not a directory" + + +def test_run_writes_to_the_destination_the_cli_handed_it(): + # The actual dispatch path: `quantprobe fetch org/repo `. + with tempfile.TemporaryDirectory() as tmp: + dest = os.path.join(tmp, "weights") + a = argparse.Namespace( + repo="org/repo", dest=dest, files=["UD-Q2_K_XL/model.gguf"], force=False + ) + with synthetic_http(), mock.patch.object(fmod, "token", lambda: None): + try: + fmod.run(a) + except SystemExit as e: + assert e.code == 0, f"fetch.run exited {e.code}, expected 0" + else: + raise AssertionError("fetch.run must exit") + assert os.path.isfile(os.path.join(dest, "UD-Q2_K_XL", "model.gguf")) + + +def run_smoke(): + """Run every destination regression without requiring pytest.""" + ran = 0 + for name, fn in sorted(globals().items()): + if name.startswith("test_"): + try: + fn() + except Exception as exc: + raise AssertionError(f"{name}: {exc}") from exc + ran += 1 + assert ran == 6, f"expected 6 destination cases, ran {ran}"