Skip to content

common: refactor model handling #24980

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ngxson merged 12 commits into from
Jun 25, 2026
Merged

common: refactor model handling #24980

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ac9d8be
common: refactor models handling
ngxson Jun 23, 2026
2188cc7
remote preset
ngxson Jun 23, 2026
5fcd847
cont
ngxson Jun 24, 2026
0bc03bb
rm skip_download option
ngxson Jun 24, 2026
206b378
missing header
ngxson Jun 24, 2026
89de8da
fix plan.model_files
ngxson Jun 24, 2026
4079ce6
fix --offline case
ngxson Jun 24, 2026
df822d2
move hf_plan to download
ngxson Jun 24, 2026
6244ba7
refactor
ngxson Jun 25, 2026
c75ee8a
rm redundant curr_ex, add comments
ngxson Jun 25, 2026
89e37a5
Merge branch 'master' into xsn/refactor_common_models
ngxson Jun 25, 2026
c705582
adapt
ngxson Jun 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
278 changes: 174 additions & 104 deletions common/arg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,60 +297,6 @@ struct handle_model_result {
std::string preset_path;
};

static handle_model_result common_params_handle_model(struct common_params_model & model,
const common_download_opts & opts) {
handle_model_result result;

// TODO @ngxson : refactor this into a new common_model_download_context

if (!model.docker_repo.empty()) {
model.path = common_docker_resolve_model(model.docker_repo);
} else if (!model.hf_repo.empty()) {
// If -m was used with -hf, treat the model "path" as the hf_file to download
if (model.hf_file.empty() && !model.path.empty()) {
model.hf_file = model.path;
model.path = "";
}
common_download_opts hf_opts = opts;
auto download_result = common_download_model(model, hf_opts);

if (!download_result.preset_path.empty()) {
result.found_preset = true;
result.preset_path = download_result.preset_path;
return result; // skip everything else if preset.ini is used
}

if (download_result.model_path.empty()) {
throw std::runtime_error("failed to download model from Hugging Face");
}

model.path = download_result.model_path;

if (!download_result.mmproj_path.empty()) {
result.found_mmproj = true;
result.mmproj.path = download_result.mmproj_path;
}

if (!download_result.mtp_path.empty()) {
result.found_mtp = true;
result.mtp.path = download_result.mtp_path;
}
} else if (!model.url.empty()) {
if (model.path.empty()) {
auto f = string_split<std::string>(model.url, '#').front();
f = string_split<std::string>(f, '?').front();
model.path = fs_get_cache_file(string_split<std::string>(f, '/').back());
}

auto download_result = common_download_model(model, opts);
if (download_result.model_path.empty()) {
throw std::runtime_error("failed to download model from " + model.url);
}
}

return result;
}

const std::vector<ggml_type> kv_cache_types = {
GGML_TYPE_F32,
GGML_TYPE_F16,
Expand Down Expand Up @@ -395,77 +341,199 @@ static bool parse_bool_value(const std::string & value) {
}

//
// CLI argument parsing functions
// common_models_handler
//

bool common_params_handle_models(common_params & params, llama_example curr_ex, const common_params_handle_models_params & handle_params) {
void common_models_handler::fetch_meta(llama_example curr_ex) {
const bool spec_type_draft_mtp = std::find(params.speculative.types.begin(),
params.speculative.types.end(),
COMMON_SPECULATIVE_TYPE_DRAFT_MTP) != params.speculative.types.end();
params.speculative.types.end(),
COMMON_SPECULATIVE_TYPE_DRAFT_MTP) != params.speculative.types.end();

// only download mmproj if the current example is using it
bool use_mmproj = false;
for (const auto & ex : mmproj_examples) {
if (curr_ex == ex) {
use_mmproj = true;
break;
}
}

common_download_opts opts;
opts.bearer_token = params.hf_token;
opts.offline = params.offline;
opts.skip_download = params.skip_download;
opts.download_mtp = spec_type_draft_mtp;
opts.download_mmproj = !params.no_mmproj && params.mmproj.path.empty() && params.mmproj.url.empty();
opts.preset_only = handle_params.preset_only;
opts.download_mmproj = use_mmproj && !params.no_mmproj
&& params.mmproj.path.empty() && params.mmproj.url.empty();

if (handle_params.callback) {
opts.callback = handle_params.callback;
if (callback) {
opts.callback = callback;
}

// sub-models (draft, mmproj, vocoder) are explicitly specified by the user,
// so we should not auto-discover mtp/mmproj siblings for them
common_download_opts sub_opts = opts;
sub_opts.download_mtp = false;
sub_opts.download_mmproj = false;
if (!params.model.hf_repo.empty()) {
plan = get_hf_plan(params.model, opts);
}

try {
auto res = common_params_handle_model(params.model, opts);
if (res.found_preset) {
if (!params.models_preset.empty()) {
throw std::invalid_argument("cannot use both --models-preset and -hf with a preset.ini file");
// handle plain "url" if needed
auto handle_url = [&](common_params_model & model) {
if (!model.url.empty()) {
if (model.path.empty()) {
model.path = get_default_local_path(model.url);
}
// if HF repo is a preset repo, we simply run server in router mode with the preset.ini file
params.models_preset_hf = params.model.hf_repo; // only for showing a warning
params.models_preset = res.preset_path;
params.model = common_params_model{}; // make sure to clear model, so server starts in router mode
return true;
}
};
handle_url(params.model);
handle_url(params.mmproj);
handle_url(params.vocoder.model);
handle_url(params.speculative.draft.mparams);

// optionally, if docker repo is set, resolve it
if (!params.model.docker_repo.empty()) {
params.model.url = common_docker_resolve_model(params.model.docker_repo);
params.model.path = get_default_local_path(params.model.url);
}
}

bool common_models_handler::is_preset_repo() const {
return !plan.preset.url.empty();
}

void common_models_handler::apply() {
std::vector<common_download_task> tasks;

if (params.no_mmproj) {
params.mmproj = {};
} else if (res.found_mmproj && params.mmproj.path.empty() && params.mmproj.url.empty()) {
// optionally, handle mmproj model when -hf is specified
params.mmproj = res.mmproj;
// handle plain "url" tasks (non-hf)
if (!params.model.url.empty()) {
auto url_tasks = build_url_tasks(params.model);
// the first part is what gets loaded, so point params.model.path at it
if (!url_tasks.empty()) {
std::string first_path = url_tasks.front().local_path;
url_tasks.front().on_done = [&]() { params.model.path = first_path; };
}
// only download mmproj if the current example is using it
for (const auto & ex : mmproj_examples) {
if (curr_ex == ex) {
common_params_handle_model(params.mmproj, sub_opts);
break;
for (auto & task : url_tasks) {
tasks.push_back(std::move(task));
}
}
if (!params.mmproj.url.empty()) {
common_download_task task;
task.url = params.mmproj.url;
task.local_path = params.mmproj.path;
task.opts = opts;
tasks.push_back(task);
}
if (!params.vocoder.model.url.empty()) {
common_download_task task;
task.url = params.vocoder.model.url;
task.local_path = params.vocoder.model.path;
task.opts = opts;
tasks.push_back(task);
}
if (!params.speculative.draft.mparams.url.empty()) {
common_download_task task;
task.url = params.speculative.draft.mparams.url;
task.local_path = params.speculative.draft.mparams.path;
task.opts = opts;
tasks.push_back(task);
}

// handle hf_plan tasks
if (!plan.model_files.empty()) {
for (size_t i = 0; i < plan.model_files.size(); ++i) {
auto & model_file = plan.model_files[i];
bool is_first = (i == 0);
tasks.emplace_back(model_file, opts, [&, is_first]() {
if (is_first) {
// only use first part as model path
params.model.path = hf_cache::finalize_file(model_file);
} else {
hf_cache::finalize_file(model_file);
}
});
}
}
if (!plan.mmproj.local_path.empty()) {
tasks.emplace_back(plan.mmproj, opts, [&]() {
params.mmproj.path = hf_cache::finalize_file(plan.mmproj);
});
}
if (!plan.mtp.local_path.empty()) {
tasks.emplace_back(plan.mtp, opts, [&]() {
// only fall back to the discovered MTP head when no draft was explicitly provided
if (params.speculative.draft.mparams.empty()) {
params.speculative.draft.mparams.path = hf_cache::finalize_file(plan.mtp);
} else {
hf_cache::finalize_file(plan.mtp);
}
});
}
if (!plan.preset.local_path.empty()) {
tasks.emplace_back(plan.preset, opts, [&]() {
// if HF repo is a preset repo, we simply run server in router mode with the preset.ini file
params.models_preset_hf = params.model.hf_repo; // only for showing a warning
params.models_preset = hf_cache::finalize_file(plan.preset);
params.model = common_params_model{}; // make sure to clear model, so server starts in router mode
});
}

// run all tasks in parallel
if (!params.offline) {
common_download_run_tasks(tasks);
}

// download successful, update params with the downloaded paths
for (const auto & task : tasks) {
if (task.on_done) {
task.on_done();
}
}
}

// when --spec-type mtp is set and no draft model was provided explicitly,
// fall back to the MTP head discovered alongside the -hf model
if (spec_type_draft_mtp && res.found_mtp &&
params.speculative.draft.mparams.path.empty() &&
params.speculative.draft.mparams.hf_repo.empty() &&
params.speculative.draft.mparams.url.empty()) {
params.speculative.draft.mparams.path = res.mtp.path;
std::string common_models_handler::get_default_local_path(const std::string & url) {
auto f = string_split<std::string>(url, '#').front();
f = string_split<std::string>(f, '?').front();
return fs_get_cache_file(string_split<std::string>(f, '/').back());
}

std::vector<common_download_task>
common_models_handler::build_url_tasks(const common_params_model & model) {
auto parts = common_download_get_all_parts(model.url);
std::vector<common_download_task> tasks;

// single-part: download straight to model.path if the user gave one (-m), else the cache default
if (parts.size() == 1) {
common_download_task task;
task.url = parts[0];
task.local_path = model.path.empty() ? get_default_local_path(parts[0]) : model.path;
task.opts = opts;
tasks.push_back(std::move(task));
return tasks;
}

// multi-part: place each part under the user's -m directory (if given), else the cache default
std::string base_dir;
if (!model.path.empty()) {
auto pos = model.path.rfind('/');
base_dir = pos == std::string::npos ? std::string(".") : model.path.substr(0, pos);
}

for (const auto & part : parts) {
common_download_task task;
task.url = part;
task.opts = opts;

std::string local = get_default_local_path(part);
if (!base_dir.empty()) {
auto pos = local.rfind('/');
std::string name = pos == std::string::npos ? local : local.substr(pos + 1);
local = base_dir + "/" + name;
}
common_params_handle_model(params.speculative.draft.mparams, sub_opts);
common_params_handle_model(params.vocoder.model, sub_opts);
return true;
} catch (const common_skip_download_exception &) {
return false;
} catch (const std::exception &) {
throw;
task.local_path = local;
tasks.push_back(std::move(task));
}
return tasks;
}

//
// CLI argument parsing functions
//

static bool common_params_parse_ex(int argc, char ** argv, common_params_context & ctx_arg) {
common_params & params = ctx_arg.params;

Expand Down Expand Up @@ -599,7 +667,9 @@ static bool common_params_parse_ex(int argc, char ** argv, common_params_context

if (!skip_model_download) {
// handle model and download
common_params_handle_models(params, ctx_arg.ex, {});
common_models_handler handler(params);
handler.fetch_meta(ctx_arg.ex);
handler.apply();

// model is required (except for server)
// TODO @ngxson : maybe show a list of available models in CLI in this case
Expand Down
33 changes: 22 additions & 11 deletions common/arg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include "common.h"
#include "download.h"
#include "hf-cache.h"

@ggerganov ggerganov Jun 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed


#include <set>
#include <map>
Expand Down Expand Up @@ -130,19 +131,29 @@ bool common_params_to_map(int argc, char ** argv, llama_example ex, std::map<com
// see: https://github.com/ggml-org/llama.cpp/issues/18163
void common_params_add_preset_options(std::vector<common_arg> & args);

struct common_params_handle_models_params {
struct common_models_handler {
common_params & params;
common_download_callback * callback = nullptr;
bool preset_only = false; // if true, only check & download remote preset (for router mode)
};
hf_cache::hf_plan plan;
common_download_opts opts;

common_models_handler(common_params & params) : params(params) {}

// fetch the metadata if needed (but do not download the model)
void fetch_meta(llama_example curr_ex);

// populate model paths (main model, mmproj, etc) from -hf if necessary
// return true if the model is ready to use
// throw an exception if there is an error that prevents the model from being used (e.g. network error, model not found, etc)
// if params.skip_download is true, no downloads will be attempted. return false if the model is invalid or missing (e.g. ETag check failed)
bool common_params_handle_models(
common_params & params,
llama_example curr_ex,
const common_params_handle_models_params & handle_params);
// return true if the input -hf is a preset-only repo (i.e. contains a preset.ini file)
bool is_preset_repo() const;

// download the model if needed, then apply it to the common_params
void apply();

private:
std::string get_default_local_path(const std::string & url);

// build download tasks for a plain (non-hf) url model, honoring a user-supplied -m path
std::vector<common_download_task> build_url_tasks(const common_params_model & model);
};
Comment on lines +134 to +137

@ggerganov ggerganov Jun 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This struct seems a bit unnecessary - likely can remain a function. The private methods can be static functions in the cpp file.

@ngxson ngxson Jun 25, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sense, I refactored it again:

struct common_models_handler {
    common_download_hf_plan plan;
    common_download_opts opts;
};

common_models_handler common_models_handler_init(const common_params & params, llama_example curr_ex);
bool common_models_handler_is_preset_repo(const common_models_handler & handler);
void common_models_handler_apply(common_models_handler & handler, common_params & params, common_download_callback * callback = nullptr);

Usage:

  • common_models_handler_init will fetch metadata and construct the hf_plan / opts
  • common_models_handler_is_preset_repo is to check if the hf-repo is preset.ini-only
  • common_models_handler_apply will download the files and modify params to point to the downloaded location

@ggerganov ggerganov Jun 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since all the usages are like:

{
    auto handler = common_models_handler_init(...);
    if (need_apply) {
        common_models_handler_apply(handler);
    }
}

It does not seem necessary to introduce the common_models_handler. A single function such as the following seems enough?

void common_params_update(
        common_params & params,
        llama_example ex,
        bool apply,
        common_download_callback * callback);

@ngxson ngxson Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your idea is similar to the existing common_params_handle_models before this refactoring, but the main point of the 2-step init/apply is for server code, it looks somewhat like this:

models_handler = common_models_handler_init(params, LLAMA_EXAMPLE_SERVER);
if (common_models_handler_is_preset_repo(models_handler)) {
    // apply the preset and start the server in router mode
    common_models_handler_apply(models_handler, params);
}

// do other endpoint setup
ctx_http.get ("/models/sse", progress_download_report);
// ... then later on

if (not_preset) {
    // non-roter + non-preset: download model, with progress callback via `progress_download_report`
    common_models_handler_apply(models_handler, params, callback);
}

We could also get rid of common_models_handler but that will require re-fetching the list of files for each call. My idea is somewhat like this:

  • The list of files to be downloaded will be stored in common_models_handler
  • It can also be useful for future, for example, if we want to show to the user list of files (+ size), then ask for confirmation before downloading

@ngxson ngxson Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, note that if we only call common_models_handler_init without _apply, that means "validate if the HF model exist", and indeed the logic is being used in POST /models endpoint is now benefit from this


// initialize argument parser context - used by test-arg-parser and preset
common_params_context common_params_parser_init(common_params & params, llama_example ex, void(*print_usage)(int, char **) = nullptr);
Loading
Loading