From f98557ac87d519ff81fa44c7a761da9a25e6c0e6 Mon Sep 17 00:00:00 2001 From: Divyam Talwar Date: Sun, 20 Sep 2026 03:09:49 +0530 Subject: [PATCH] fix(auto): retain nested download paths in downstream handoffs Address #19 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 | 6 + quantprobe/auto.py | 20 ++- tests/smoke.py | 27 ++++ tests/test_auto_nested_paths.py | 268 ++++++++++++++++++++++++++++++++ 4 files changed, 314 insertions(+), 7 deletions(-) create mode 100644 tests/test_auto_nested_paths.py diff --git a/QUICKSTART.md b/QUICKSTART.md index 69b20a4..e4dd4bc 100644 --- a/QUICKSTART.md +++ b/QUICKSTART.md @@ -41,6 +41,12 @@ before committing you to anything over two hours. You can stop between stages; n and `--custom` will decline and explain why. Custom pays when you're squeezing a model that barely fits, or when it's your own fine-tune that nobody has published. +**Where the file lands.** Both paths download into `--dir` (default `./models`) keeping the +repo's own layout, so a quant published at `Q2_K/Model-Q2_K.gguf` arrives at +`./models/Q2_K/Model-Q2_K.gguf`. Every path `auto` prints afterwards — the `run` command, the +`quantize` shortcut — is that full path, copy-pasteable as printed; and a re-run finds the file +already on disk and predicts from its real header instead of from preset estimates. + ## The free speed most people miss If you run a mixture-of-experts model (Qwen3-30B-A3B, GLM-Air, most big local models), the usual diff --git a/quantprobe/auto.py b/quantprobe/auto.py index 91d0d2a..6d3487e 100644 --- a/quantprobe/auto.py +++ b/quantprobe/auto.py @@ -490,6 +490,11 @@ def run(a): sbits, ssize, spath = src dest = getattr(a, "dir", None) or "./models" os.makedirs(dest, exist_ok=True) + # Where the fetch below actually puts the file: `fetch` writes dest/, and + # big repos keep their quants in subfolders (Q8_0/...). Rebuilt from the basename, this + # named a file that does not exist - for the probe input AND for the command printed + # above it. One expression, so the two cannot drift apart again. + srcfull = os.path.join(dest, spath) print( "\n[quantprobe auto --custom] source: " + spath @@ -513,10 +518,7 @@ def run(a): print(" skip the probe below and build straight from it (minutes, not hours):") # The path the fetch below will actually produce, not the bare repo filename - a # command the user cannot paste yet is worse than no command. - print( - f" quantprobe quantize --gguf " - f"{os.path.join(dest, os.path.basename(spath))} --recipe {target}" - ) + print(f" quantprobe quantize --gguf {srcfull} --recipe {target}") print(" Continuing re-measures it on YOUR file, which is the right call if your") print(" source differs from the one above: the band is a property of the weights,") print(" not of the name. If it is the same source, you are paying twice.\n") @@ -530,12 +532,13 @@ def run(a): if not fetchmod.fetch(repo, dest, spath, fetchmod.token()): raise SystemExit("source download failed (re-run: it resumes)") - srcfull = os.path.join(dest, os.path.basename(spath)) evalf = ensure_eval(dest) import argparse from . import probe as probemod + # Basename here is deliberate: this artifact is BUILT locally, so it has no remote + # directory to preserve - it belongs at the top of --dir, next to nothing. out = os.path.join(dest, os.path.basename(spath).rsplit(".gguf", 1)[0] + "-depthaware.gguf") pa = argparse.Namespace( gguf=srcfull, @@ -583,7 +586,10 @@ def run(a): # `auto` confidently prints a number derived from a model that is not there. A crash # is recoverable; a confident wrong answer is the defect class this project exists to # remove. The size check below is what catches (b), and it is the more important half. - _local = os.path.join(getattr(a, "dir", None) or "./models", os.path.basename(path)) + # `path` is the REPO-RELATIVE path and `fetch` writes dest/, subfolders included. Built + # from the basename this looked one directory too high, so a finished download in a repo that + # nests its quants was never found and the pre-download estimate was quoted instead. + _local = os.path.join(getattr(a, "dir", None) or "./models", path) _s, _why = local_spec_or_none(_local, size, len(parts)) if _why: print(" NOTE: " + _why) @@ -637,7 +643,7 @@ def run(a): print(f"[quantprobe auto] part {i + 1}/{len(parts)}: {os.path.basename(part)}") if not fetchmod.fetch(repo, dest, part, fetchmod.token()): raise SystemExit("download failed (it resumes: re-run the same command)") - full = os.path.join(dest, os.path.basename(path)) + full = os.path.join(dest, path) # what fetch just wrote, subdirectories and all print("\n[quantprobe auto] ready. Run it:") print(f" quantprobe run --gguf {full}") print("\n Better quality at the SAME size: rerun with --custom - it probes YOUR model's") diff --git a/tests/smoke.py b/tests/smoke.py index fce0397..cf38494 100644 --- a/tests/smoke.py +++ b/tests/smoke.py @@ -2818,6 +2818,33 @@ def t_auto_never_trusts_an_incomplete_local_gguf(): assert local_spec_or_none(os.path.join(d, "nope.gguf"), 123, 1) == (None, None) return None +def t_auto_keeps_the_remote_subdirectory_of_every_download(): + """`auto` must hand downstream the path `fetch` actually wrote, not dest/basename. + + Large GGUF repos keep their quants in subfolders, `fetch` writes dest/, + and `auto` rebuilt every path after the download from the basename alone. The download + succeeded and the run handoff, the --run launch, the --custom probe input, the pasteable + quantize command and the "already on disk" header read all named a file one directory too + high. Behavioural, not source-text: the fake download writes sentinel bytes to the real + destination and the cases assert the handed-off file EXISTS. + + Shares its scenarios with tests/test_auto_nested_paths.py so the pytest file and this suite + cannot drift. Mutation owed and discharged: restoring os.path.basename at any of the four + input-file call sites in auto.run fails this. + """ + import importlib.util, tempfile + here = os.path.dirname(os.path.abspath(__file__)) + spec = importlib.util.spec_from_file_location( + "test_auto_nested_paths", os.path.join(here, "test_auto_nested_paths.py")) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + assert len(mod.CASES) >= 7, f"only {len(mod.CASES)} nested-path cases - did the file shrink?" + for case in mod.CASES: + with tempfile.TemporaryDirectory() as d: + case(d) # no skip branch: every case runs, always + return None + + def t_ollama_eval_rate_is_generation_not_prompt(): """audit-ollama must never read the PROMPT rate as the generation rate. diff --git a/tests/test_auto_nested_paths.py b/tests/test_auto_nested_paths.py new file mode 100644 index 0000000..9242be2 --- /dev/null +++ b/tests/test_auto_nested_paths.py @@ -0,0 +1,268 @@ +"""`auto` must keep the remote subdirectory of every file it downloads. + +`fetch(repo, dest, fname)` writes `os.path.join(dest, fname)` - so a repo that keeps its quants +in subfolders (`Q2_K/Model-Q2_K.gguf`, the layout every large GGUF repo uses) lands on disk at +`dest/Q2_K/Model-Q2_K.gguf`. `auto` then rebuilt that path as `dest/basename(path)`, which is a +file that does not exist: the run handoff, the `--run` launch, the `--custom` probe input, the +pasteable `quantize` command and the "already on disk" header read all pointed one directory too +high. The download succeeded and every path printed after it was wrong. + +These are behavioural, not source-text checks. The fake download writes a sentinel byte string to +the REAL destination `fetch` would write to, and the assertions ask whether the file `auto` hands +downstream actually exists. Only the network (`list_ggufs`, `fetch`), the GGUF header reader and +the probe/runtime subprocesses are faked; every path expression under test runs for real. + +Deliberately NOT covered here: `fetch` creating the missing parent directory. That is a separate +defect with its own tests - these cases pre-create the subdirectory, so they fail on the path +bookkeeping alone and stay red whether or not `fetch` learned to mkdir. + +Each scenario is a plain `_case_*(tmpdir)` function so `tests/smoke.py` can run the same +regressions without pytest. +""" + +from __future__ import annotations + +import contextlib +import io +import os +import sys +from types import SimpleNamespace +from unittest import mock + +REPO = "acme/Nested-GGUF" +TOTAL_B = 0.01 # 10M params: keeps the exact-size fixture file kilobytes, not gigabytes +HW = ["--vram", "24", "--vram-bw", "900", "--ram", "64", "--ram-bw", "80", "--disk-bw", "3"] + +NESTED_Q2 = "Q2_K/Model-Q2_K.gguf" +NESTED_Q8 = "Q8_0/Model-Q8_0.gguf" +NESTED_SPLIT_1 = "Q2_K/Model-Q2_K-00001-of-00002.gguf" +NESTED_SPLIT_2 = "Q2_K/Model-Q2_K-00002-of-00002.gguf" +FLAT_Q4 = "Model-Q4_K_M.gguf" + +#: What the faked header reader returns, shaped like `spec.from_gguf`. +SPEC = { + "t": TOTAL_B, + "a": TOTAL_B, + "ne": TOTAL_B, + "moe": False, + "bits": 2.5, + "kvp": 98304.0, + "n_layer": 24, + "codebook_share": 0.0, +} + + +def _size(bits, total_b=TOTAL_B): + """Bytes a file must declare for `auto` to read it back as `bits` effective bits.""" + return round(bits * total_b * 1e9 / 8) + + +def _drive(tmp, target, extra, files, *, local=None): + """Run `quantprobe auto ...` through cli.main with every external boundary faked. + + Returns what the real path expressions produced: which files were fetched, what `--run` and + the probe were handed, and whether those paths existed when they were handed over. + """ + from quantprobe import auto as automod + from quantprobe import cli as climod + from quantprobe import fetch as fetchmod + from quantprobe import probe as probemod + from quantprobe import runtime as rtmod + from quantprobe import spec as specmod + + dest = os.path.join(tmp, "models") + os.makedirs(dest, exist_ok=True) + # The subdirectories a previous download already left behind, so nothing here depends on + # fetch() creating them (that is the sibling defect, tested separately). + for path, _ in files: + sub = os.path.dirname(path) + if sub: + os.makedirs(os.path.join(dest, sub), exist_ok=True) + if local: + name, size = local + with open(os.path.join(dest, name), "wb") as fh: + fh.write(b"GGUF") + fh.truncate(size) + + rec = SimpleNamespace( + dest=dest, + fetched=[], + run_gguf=None, + run_existed=None, + probe_args=None, + probe_src_existed=None, + out="", + ) + + def fake_fetch(repo, dst, fname, tok, tries=100, force=False): + rec.fetched.append((repo, dst, fname)) + with open(os.path.join(dst, fname), "wb") as fh: # exactly where fetch() writes + fh.write(b"GGUF sentinel") + return True + + def fake_runtime_run(a): + rec.run_gguf = a.gguf + rec.run_existed = bool(a.gguf) and os.path.isfile(a.gguf) + + def fake_probe_run(pa): + rec.probe_args = pa + rec.probe_src_existed = os.path.isfile(pa.gguf) + with open(pa.out, "wb") as fh: + fh.write(b"GGUF depth-aware sentinel") + + def fake_ensure_eval(d): + p = os.path.join(d, "wiki.test.raw") + with open(p, "w", encoding="utf-8") as fh: + fh.write("held-out text\n") + return p + + argv = ["quantprobe", "auto", target, "--dir", dest] + HW + list(extra) + buf = io.StringIO() + with contextlib.ExitStack() as st: + st.enter_context(mock.patch.object(automod, "list_ggufs", lambda repo: list(files))) + st.enter_context(mock.patch.object(automod, "ensure_eval", fake_ensure_eval)) + st.enter_context(mock.patch.object(fetchmod, "fetch", fake_fetch)) + st.enter_context(mock.patch.object(fetchmod, "token", lambda: None)) + st.enter_context(mock.patch.object(probemod, "run", fake_probe_run)) + st.enter_context(mock.patch.object(rtmod, "run", fake_runtime_run)) + st.enter_context(mock.patch.object(specmod, "from_gguf", lambda p: dict(SPEC))) + st.enter_context(mock.patch.object(sys, "argv", argv)) + st.enter_context(contextlib.redirect_stdout(buf)) + climod.main() + rec.out = buf.getvalue() + return rec + + +def _case_standard_route_hands_off_the_downloaded_file(tmp): + """`auto --run`: the file handed to the runtime must be the one fetch wrote.""" + rec = _drive(tmp, REPO, ["--total", str(TOTAL_B), "--run"], [(NESTED_Q2, _size(2.5))]) + want = os.path.join(rec.dest, "Q2_K", "Model-Q2_K.gguf") + + assert rec.fetched == [(REPO, rec.dest, NESTED_Q2)], rec.fetched + assert os.path.isfile(want), "the fake download did not land where fetch() writes" + assert rec.run_gguf == want, f"--run was handed {rec.run_gguf!r}, fetch wrote {want!r}" + assert rec.run_existed, f"--run was handed a path that does not exist: {rec.run_gguf!r}" + assert f"quantprobe run --gguf {want}" in rec.out, ( + "the printed ready command names a file that was never written:\n" + rec.out[-500:] + ) + + +def _case_split_first_part_handoff(tmp): + """A split download: every part lands under the subfolder, and the handoff is part 1.""" + rec = _drive(tmp, REPO, ["--total", str(TOTAL_B), "--run"], [(NESTED_SPLIT_1, _size(2.5))]) + want = os.path.join(rec.dest, "Q2_K", "Model-Q2_K-00001-of-00002.gguf") + + assert rec.fetched == [ + (REPO, rec.dest, NESTED_SPLIT_1), + (REPO, rec.dest, NESTED_SPLIT_2), + ], rec.fetched + assert os.path.isfile(want) + assert rec.run_gguf == want, f"--run was handed {rec.run_gguf!r}, fetch wrote {want!r}" + assert rec.run_existed, f"split handoff points at a missing file: {rec.run_gguf!r}" + assert f"quantprobe run --gguf {want}" in rec.out, rec.out[-500:] + + +def _case_flat_remote_path_is_unchanged(tmp): + """Control: a repo-root file has no subdirectory to lose, and must behave exactly as before.""" + rec = _drive(tmp, REPO, ["--total", str(TOTAL_B), "--run"], [(FLAT_Q4, _size(4.5))]) + want = os.path.join(rec.dest, FLAT_Q4) + + assert rec.fetched == [(REPO, rec.dest, FLAT_Q4)], rec.fetched + assert rec.run_gguf == want and rec.run_existed + assert f"quantprobe run --gguf {want}" in rec.out, rec.out[-500:] + + +def _case_custom_route_probes_the_downloaded_source(tmp): + """`--custom`: the probe input is the fetched high-precision source, not a phantom sibling.""" + files = [(NESTED_Q8, _size(8.0)), (NESTED_Q2, _size(2.5))] + rec = _drive(tmp, REPO, ["--total", str(TOTAL_B), "--custom", "--force-custom"], files) + src = os.path.join(rec.dest, "Q8_0", "Model-Q8_0.gguf") + + assert rec.fetched == [(REPO, rec.dest, NESTED_Q8)], rec.fetched + assert rec.probe_args is not None, "the probe never ran" + assert rec.probe_args.gguf == src, f"probe input {rec.probe_args.gguf!r}, fetch wrote {src!r}" + assert rec.probe_src_existed, f"probe was handed a missing file: {rec.probe_args.gguf!r}" + # Derived output naming for the NEW artifact stays flat in --dir: it is built here, not + # fetched, so it has no remote directory to preserve. + out = os.path.join(rec.dest, "Model-Q8_0-depthaware.gguf") + assert rec.probe_args.out == out, rec.probe_args.out + assert f"quantprobe run --gguf {out}" in rec.out, rec.out[-500:] + + +def _case_custom_atlas_hint_quotes_a_pasteable_path(tmp): + """The atlas shortcut prints the path the fetch below WILL produce - so it must be that one.""" + rec = _drive( + tmp, "qwen3-30b", ["--custom", "--force-custom", "--dry"], [(NESTED_Q8, _size(8.0, 30.5))] + ) + want = os.path.join(rec.dest, "Q8_0", "Model-Q8_0.gguf") + + assert rec.fetched == [], "--dry downloaded something" + assert f"quantprobe quantize --gguf {want} --recipe qwen3-30b" in rec.out, ( + "the atlas shortcut prints a command the user cannot paste:\n" + rec.out[-800:] + ) + + +def _case_existing_local_file_is_found_under_its_remote_path(tmp): + """A completed download already on disk must be read, not missed and re-estimated.""" + size = _size(2.5) + rec = _drive( + tmp, REPO, ["--total", str(TOTAL_B), "--dry"], [(NESTED_Q2, size)], local=(NESTED_Q2, size) + ) + + assert rec.fetched == [], "--dry downloaded something" + assert "[from the file's own header - already on disk]" in rec.out, ( + "the file on disk was not found, so auto quoted a pre-download estimate:\n" + rec.out[-800:] + ) + assert "NOTE: pre-download estimate from preset params" not in rec.out, rec.out[-800:] + assert "INCOMPLETE" not in rec.out, rec.out[-800:] + + +def _case_dry_downloads_nothing(tmp): + """`--dry` still names the remote path in full, and touches the network for no bytes.""" + rec = _drive(tmp, REPO, ["--total", str(TOTAL_B), "--dry"], [(NESTED_Q2, _size(2.5))]) + + assert rec.fetched == [], "--dry downloaded something" + assert rec.run_gguf is None, "--dry launched the runtime" + assert NESTED_Q2 in rec.out, rec.out[-500:] + assert "(--dry: nothing downloaded)" in rec.out, rec.out[-500:] + assert not os.path.exists(os.path.join(rec.dest, NESTED_Q2)) + + +#: Shared with tests/smoke.py so the suite runs these regressions without pytest. +CASES = [ + _case_standard_route_hands_off_the_downloaded_file, + _case_split_first_part_handoff, + _case_flat_remote_path_is_unchanged, + _case_custom_route_probes_the_downloaded_source, + _case_custom_atlas_hint_quotes_a_pasteable_path, + _case_existing_local_file_is_found_under_its_remote_path, + _case_dry_downloads_nothing, +] + + +def test_standard_route_hands_off_the_downloaded_file(tmp_path): + _case_standard_route_hands_off_the_downloaded_file(str(tmp_path)) + + +def test_split_first_part_handoff(tmp_path): + _case_split_first_part_handoff(str(tmp_path)) + + +def test_flat_remote_path_is_unchanged(tmp_path): + _case_flat_remote_path_is_unchanged(str(tmp_path)) + + +def test_custom_route_probes_the_downloaded_source(tmp_path): + _case_custom_route_probes_the_downloaded_source(str(tmp_path)) + + +def test_custom_atlas_hint_quotes_a_pasteable_path(tmp_path): + _case_custom_atlas_hint_quotes_a_pasteable_path(str(tmp_path)) + + +def test_existing_local_file_is_found_under_its_remote_path(tmp_path): + _case_existing_local_file_is_found_under_its_remote_path(str(tmp_path)) + + +def test_dry_downloads_nothing(tmp_path): + _case_dry_downloads_nothing(str(tmp_path))