Skip to content

server-context: chain the caller-provided load_progress_callback instead of discarding it#25306

Closed
mtmcp wants to merge 1 commit into
ggml-org:masterfrom
mtmcp:server-chain-progress-callback
Closed

server-context: chain the caller-provided load_progress_callback instead of discarding it#25306
mtmcp wants to merge 1 commit into
ggml-org:masterfrom
mtmcp:server-chain-progress-callback

Conversation

@mtmcp

@mtmcp mtmcp commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

Overview

In order to update llama-cli model-loading capability (for instance, swapping models with a /model slash-command), model-loading should be able to be gracefully cancelled without exiting the application.

At the moment, server_context_impl::load_model() unconditionally overwrites params.load_progress_callback/_user_data with its own static load_progress_callback (pointed at its internal load_progress_data) before calling common_init_from_params():

params_base.load_progress_callback           = load_progress_callback;
params_base.load_progress_callback_user_data = &load_progress_text;

As a result, any embedder that links server-context and sets common_params.load_progress_callback to observe or cancel a model load has it silently dropped — the model loader only ever calls the engine's own callback. During llama_model_load the progress callback's false return is the only cancellation hook (the llama_context abort callback does not exist until after the load), so this effectively makes an in-progress load uncancellable for engine embedders, and llama-cli today sets no load callback at all.

This change captures the caller's {callback, user_data} at the top of load_model() and invokes it from the
engine's static callback, before the engine's own throttled state reporting:

if (d->orig_cb && !d->orig_cb(progress, d->orig_ud)) {
    return false;   // caller aborted the load
}

Compatibility

Strictly additive. When no caller callback is supplied (orig_cb == nullptr) behavior is identical to today;
existing callback_state / SERVER_STATE_LOADING reporting is unchanged.

Notes
• The caller's callback is invoked per load stage (text / mmproj / spec), since each stage runs its own
load_progress_data. A cancellation predicate is unaffected; a progress reporter should expect one 0→1 sweep
per stage.
• It is called before the engine's 200 ms callback_state throttle, so a cancel is not delayed up to 200 ms.
The loader does not call it in a hot loop, so this is cheap.

Additional information

Requirements

@mtmcp mtmcp requested a review from a team as a code owner July 4, 2026 18:28
@github-actions github-actions Bot added the server label Jul 4, 2026
@ggml-gh-bot

ggml-gh-bot Bot commented Jul 4, 2026

Copy link
Copy Markdown

Hi @mtmcp, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

  • AI-generated content: This project does not accept PRs, descriptions or commit messages that are fully or predominantly AI-generated. If you have used AI to assist you in writing code, please make sure to disclose that explicitly.

Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

@ngxson

ngxson commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

(for instance, swapping models with a /model slash-command)

llama-cli will soon be a http client, this fix is obsolete

@ngxson ngxson closed this Jul 5, 2026
@mtmcp

mtmcp commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

Are there any open discussions on this? I understand some of the merits of that decision from a maintenance standpoint, but then why will folks even use llama-cli if they can just use one of the many existing platform-agnostic, thin-client experiences out there? I guess if you have it small enough like a nano-client with the express purpose of quickly validating functionality on the server it makes sense.

I will go a step outside the implementation, and ask, "why do we need llama-cli?" and think in terms of a need (not a feature). What is the reason for its existence? I'm curious to know that, as it is somewhat elusive to me at this point.

@mtmcp

mtmcp commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

I see the recent discussions on #24948. This clarifies things and I see the HTTP dependency a stepping-stone (possibly).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants