From 1c53e05aeecbbe959da6aa5c12df93586268a595 Mon Sep 17 00:00:00 2001 From: Dominic Dones Date: Thu, 2 Jul 2026 16:35:52 -0400 Subject: [PATCH] examples: make -hf resolve a cached model without --hf-file Change the empty --hf-file branch of whisper_hf_resolve_model to be cache-first and refuse ambiguity rather than pick the repo's first ggml-*.bin. With no -hff: exactly one cached ggml-*.bin resolves it (no network); more than one errors and lists the cached files; a cold cache errors and lists the repo's available models instead of silently downloading. This makes "download once with -hff, then just -hf" work. Unlike llama.cpp's -hf /[:quant] default-quant pick (find_best_model), whisper repos are many-models-one-repo with no meaningful default, so we key off the cache and error+list on ambiguity. The explicit -hff path (download-first, cache fall-back) is unchanged. whisper_hf_resolve_model now prints a specific diagnostic for every failure mode, so cli.cpp no longer prints its own generic (and now inaccurate) "not found in HF cache" line; it just returns exit 3. tests/test-hf-resolve.sh gains single-cached (-hf alone -> exit 0) and multi-cached (-hf alone -> exit 3 + "multiple models cached" + list) cases, and the missing-file assertion matches the new message. Co-Authored-By: Claude Opus 4.8 --- examples/cli/cli.cpp | 3 +- examples/common-whisper.cpp | 67 +++++++++++++++++++++++++++++++++++-- tests/test-hf-resolve.sh | 31 +++++++++++++++-- 3 files changed, 94 insertions(+), 7 deletions(-) diff --git a/examples/cli/cli.cpp b/examples/cli/cli.cpp index 9bc41059b..f5165320a 100644 --- a/examples/cli/cli.cpp +++ b/examples/cli/cli.cpp @@ -1080,8 +1080,7 @@ int main(int argc, char ** argv) { if (!params.hf_repo.empty() && params.model == "models/ggml-base.en.bin") { params.model = whisper_hf_resolve_model(params.hf_repo, params.hf_file); if (params.model.empty()) { - fprintf(stderr, "error: model %s (%s) not found in HF cache\n", - params.hf_repo.c_str(), params.hf_file.c_str()); + // whisper_hf_resolve_model prints a specific diagnostic for every failure mode return 3; } } diff --git a/examples/common-whisper.cpp b/examples/common-whisper.cpp index 8a624f52a..96ca7b66d 100644 --- a/examples/common-whisper.cpp +++ b/examples/common-whisper.cpp @@ -32,6 +32,7 @@ #include #endif +#include #include #include #include @@ -246,6 +247,12 @@ bool speak_with_file(const std::string & command, const std::string & text, cons return true; } +// filename test for whisper GGML models: ggml-*.bin +static bool whisper_hf_is_ggml_bin(const std::string & name) { + return name.rfind("ggml-", 0) == 0 && name.size() >= 4 && + name.compare(name.size() - 4, 4, ".bin") == 0; +} + // pick the primary file from a listing: exact hf_file match, else the first ggml-*.bin static const hf_cache::hf_file * whisper_hf_pick_primary(const hf_cache::hf_files & files, const std::string & hf_file) { for (const auto & file : files) { @@ -255,8 +262,7 @@ static const hf_cache::hf_file * whisper_hf_pick_primary(const hf_cache::hf_file } } else { const std::string name = std::filesystem::path(file.path).filename().string(); - if (name.rfind("ggml-", 0) == 0 && name.size() >= 4 && - name.compare(name.size() - 4, 4, ".bin") == 0) { + if (whisper_hf_is_ggml_bin(name)) { return &file; } } @@ -264,6 +270,31 @@ static const hf_cache::hf_file * whisper_hf_pick_primary(const hf_cache::hf_file return nullptr; } +// collect the entries whose filename matches ggml-*.bin +static hf_cache::hf_files whisper_hf_ggml_candidates(const hf_cache::hf_files & files) { + hf_cache::hf_files out; + for (const auto & file : files) { + const std::string name = std::filesystem::path(file.path).filename().string(); + if (whisper_hf_is_ggml_bin(name)) { + out.push_back(file); + } + } + return out; +} + +// print an error message followed by the sorted list of candidate filenames +static void whisper_hf_print_candidates(const std::string & msg, const hf_cache::hf_files & candidates) { + fprintf(stderr, "%s\n", msg.c_str()); + std::vector names; + for (const auto & file : candidates) { + names.push_back(std::filesystem::path(file.path).filename().string()); + } + std::sort(names.begin(), names.end()); + for (const auto & name : names) { + fprintf(stderr, " - %s\n", name.c_str()); + } +} + std::string whisper_hf_resolve_model(const std::string & hf_repo, const std::string & hf_file) { const char * token_env = std::getenv("HF_TOKEN"); const std::string token = token_env ? token_env : ""; @@ -272,6 +303,37 @@ std::string whisper_hf_resolve_model(const std::string & hf_repo, const std::str const char * offline_env = std::getenv("HF_HUB_OFFLINE"); const bool offline = offline_env && *offline_env && std::string(offline_env) != "0"; + // -hf alone (no --hf-file): cache-first, and refuse ambiguity rather than guess. + if (hf_file.empty()) { + const hf_cache::hf_files cached = whisper_hf_ggml_candidates(hf_cache::get_cached_files(hf_repo)); + if (cached.size() == 1) { + return hf_cache::finalize_file(cached.front()); + } + if (cached.size() > 1) { + whisper_hf_print_candidates( + "error: multiple models cached for " + hf_repo + "; specify one with -hff/--hf-file:", cached); + return ""; + } + + // none cached + if (offline) { + fprintf(stderr, "error: %s not found in HF cache\n", hf_repo.c_str()); + return ""; + } + + const hf_cache::hf_files remote = whisper_hf_ggml_candidates(hf_cache::get_repo_files(hf_repo, token)); + if (remote.empty()) { + fprintf(stderr, "error: no models found in %s\n", hf_repo.c_str()); + return ""; + } + // don't auto-pick/download a multi-model repo; list what's available instead + whisper_hf_print_candidates( + "error: multiple models available in " + hf_repo + "; specify one with -hff/--hf-file:", remote); + return ""; + } + + // explicit --hf-file: download-first with cache fall-back (Phase 2, unchanged). + // 1. try download first: list the repo over the network and fetch the primary file. // get_repo_files swallows network errors into an empty result (graceful degradation). if (!offline) { @@ -289,6 +351,7 @@ std::string whisper_hf_resolve_model(const std::string & hf_repo, const std::str return hf_cache::finalize_file(*primary); } + fprintf(stderr, "error: file '%s' not found in %s (cache or network)\n", hf_file.c_str(), hf_repo.c_str()); return ""; } diff --git a/tests/test-hf-resolve.sh b/tests/test-hf-resolve.sh index 898f74995..408891421 100755 --- a/tests/test-hf-resolve.sh +++ b/tests/test-hf-resolve.sh @@ -6,7 +6,7 @@ # models--org--repo/{refs,snapshots} layout that the `hf` CLI / huggingface_hub # produces, using an existing local `for-tests` model as the payload, then checks: # 1. `-hf --hf-file ` resolves the cached snapshot and runs (exit 0) -# 2. a missing --hf-file prints the "not found in HF cache" error and exits 3 +# 2. a missing --hf-file prints the "file ... not found" error and exits 3 # 3. `-m ` regression: an explicit model path still works unchanged # 4. bare invocation (no -hf/-m) still uses the models/ggml-base.en.bin default # 5. (optional) a no-OpenSSL build attempting an https resolve with an empty @@ -65,12 +65,37 @@ fi # 2. missing file -> exit 3 with clear error HF_HUB_OFFLINE=1 HF_HUB_CACHE="$tmp_cache" "$main" -hf "$repo" --hf-file ggml-missing.bin -f "$sample" >/tmp/hf_resolve_miss.log 2>&1 rc=$? -if [ "$rc" -eq 3 ] && grep -qi "not found in HF cache" /tmp/hf_resolve_miss.log; then - printf "PASS: missing --hf-file reports 'not found in HF cache' and exits 3\n" +if [ "$rc" -eq 3 ] && grep -qi "file 'ggml-missing.bin' not found" /tmp/hf_resolve_miss.log; then + printf "PASS: missing --hf-file reports 'file ... not found' and exits 3\n" else printf "FAIL: missing --hf-file expected exit 3 + error message, got exit %s\n" "$rc"; fail=1 fi +# 2b. -hf alone (no --hf-file), single cached model -> resolves and runs (exit 0) +if HF_HUB_OFFLINE=1 HF_HUB_CACHE="$tmp_cache" "$main" -hf "$repo" -f "$sample" >/tmp/hf_resolve_single.log 2>&1; then + if grep -qi "failed to open" /tmp/hf_resolve_single.log; then + printf "FAIL: -hf alone resolved but model failed to open\n"; fail=1 + else + printf "PASS: -hf %s (no --hf-file) resolved single cached model (exit 0)\n" "$repo" + fi +else + printf "FAIL: -hf alone with a single cached model exited non-zero\n"; cat /tmp/hf_resolve_single.log; fail=1 +fi + +# 2c. -hf alone (no --hf-file), multiple cached models -> exit 3 + "multiple models cached" + both filenames +second_file="ggml-tiny.en.bin" +cp "$seed_model" "$snapshot_dir/$second_file" +HF_HUB_OFFLINE=1 HF_HUB_CACHE="$tmp_cache" "$main" -hf "$repo" -f "$sample" >/tmp/hf_resolve_multi.log 2>&1 +rc=$? +if [ "$rc" -eq 3 ] && grep -qi "multiple models cached" /tmp/hf_resolve_multi.log \ + && grep -q "$hf_file" /tmp/hf_resolve_multi.log && grep -q "$second_file" /tmp/hf_resolve_multi.log; then + printf "PASS: -hf alone with multiple cached models reports 'multiple models cached' + lists both, exits 3\n" +else + printf "FAIL: -hf alone with multiple cached models expected exit 3 + list, got exit %s\n" "$rc" + cat /tmp/hf_resolve_multi.log; fail=1 +fi +rm -f "$snapshot_dir/$second_file" + # 3. -m regression: explicit path still works if "$main" -m "$seed_model" -f "$sample" >/tmp/hf_resolve_m.log 2>&1; then printf "PASS: -m %s still works (exit 0)\n" "$seed_model"