From c620659537774597dc82cd982247688a1878e995 Mon Sep 17 00:00:00 2001 From: Brett Profitt Date: Mon, 6 Apr 2026 18:35:31 -0400 Subject: [PATCH] common: fix split model loading by sorting file list File order is not guaranteed when listing dirs. In offline mode, files are not sorted when read from cache, which can result in the wrong part loading first if the files have changed metadata (e.g., by moving, symlinking, etc). This is a minimal approach to ensure model files are correctly sorted when downloaded or loaded from cache, in online or offline mode. Tests are included. Disclaimer: An AI agent was used to refine the approach and write the test. refs: #21019 #21016 --- common/download.cpp | 9 ++++-- tests/test-gguf-model-data.cpp | 55 ++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/common/download.cpp b/common/download.cpp index 561164591fa3..4a90451a83a7 100644 --- a/common/download.cpp +++ b/common/download.cpp @@ -511,9 +511,14 @@ static hf_cache::hf_files get_split_files(const hf_cache::hf_files & files, if (split.count <= 1) { return {file}; } - hf_cache::hf_files result; - for (const auto & f : files) { + hf_cache::hf_files sorted_files = files; + std::sort(sorted_files.begin(), sorted_files.end(), [](const hf_cache::hf_file & a, const hf_cache::hf_file & b) { + return a.path < b.path; + }); + + hf_cache::hf_files result; + for (const auto & f : sorted_files) { auto split_f = get_gguf_split_info(f.path); if (split_f.count == split.count && split_f.prefix == split.prefix) { result.push_back(f); diff --git a/tests/test-gguf-model-data.cpp b/tests/test-gguf-model-data.cpp index e05c4b8976f8..c2bf677d3cdb 100644 --- a/tests/test-gguf-model-data.cpp +++ b/tests/test-gguf-model-data.cpp @@ -1,6 +1,11 @@ #include "gguf-model-data.h" +#include "common.h" +#include "download.h" +#include "hf-cache.h" #include +#include +#include #define TEST_ASSERT(cond, msg) \ do { \ @@ -10,9 +15,54 @@ } \ } while (0) +static common_download_model_result get_cached_split_files_reverse_order() { + // Offline split GGUF must return files in sequential order, not filesystem order. + namespace fs = std::filesystem; + + auto tmp = fs::temp_directory_path() / "llama-test-gguf-model-data"; + fs::remove_all(tmp); + fs::create_directories(tmp); + + const std::string commit(40, 'a'); + fs::path repo_dir = tmp / "models--testowner--testrepo"; + fs::path snap_dir = repo_dir / "snapshots" / commit; + + fs::create_directories(repo_dir / "refs"); + { std::ofstream f(repo_dir / "refs" / "main"); f << commit; } + + // Create files out of order to confirm sort works + fs::create_directories(snap_dir); + { std::ofstream f(snap_dir / "model-Q4_K_M-00002-of-00002.gguf"); } + { std::ofstream f(snap_dir / "model-Q4_K_M-00001-of-00002.gguf"); } + +#ifdef _WIN32 + _putenv_s("LLAMA_CACHE", tmp.string().c_str()); +#else + setenv("LLAMA_CACHE", tmp.string().c_str(), 1); +#endif + + common_params_model model; + model.hf_repo = "testowner/testrepo"; + + common_download_model_opts opts; + opts.offline = true; + + auto split_result = common_download_model(model, "", opts); + + fs::remove_all(tmp); +#ifdef _WIN32 + _putenv_s("LLAMA_CACHE", ""); +#else + unsetenv("LLAMA_CACHE"); +#endif + + return split_result; +} + int main() { fprintf(stderr, "=== test-gguf-model-data ===\n"); + // Fetch Qwen3-0.6B Q8_0 metadata auto result = gguf_fetch_model_meta("ggml-org/Qwen3-0.6B-GGUF", "Q8_0"); @@ -149,6 +199,11 @@ int main() { TEST_ASSERT(model4.n_vocab == 128896, "expected n_vocab == 128896"); TEST_ASSERT(model4.tensors.size() == 754, "expected tensor count == 754"); + auto split_result = get_cached_split_files_reverse_order(); + + TEST_ASSERT(!split_result.model_path.empty(), "expected non-empty model path"); + TEST_ASSERT(split_result.model_path.find("00001-of-00002") != std::string::npos, "expected first file 00001"); + fprintf(stderr, "=== ALL TESTS PASSED ===\n"); return 0; }