Skip to content

feat: add vLLM engine with on-request model switching - #181

Open
nvtushark wants to merge 41 commits into
NVIDIA:developfrom
nvtushark:vllm-engine-support
Open

nvtushark wants to merge 41 commits into
NVIDIA:developfrom
nvtushark:vllm-engine-support

Conversation

@nvtushark

@nvtushark nvtushark commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Adds vLLM as a supported engine alongside Ollama, LM Studio, and llama.cpp. PAIR installs and manages vLLM and passes requests straight to it; it does not load or unload models on its own.

  • Engine support: manifest-driven install of vLLM 0.32.0 (Linux, and Apple Silicon macOS), lifecycle, auto-sized launch settings (context length, GPU memory share, YaRN), and desktop UI.
  • Models: the model list is the Hugging Face cache, and it stays available while vLLM is stopped. Add model downloads from Hugging Face with hf, and Delete removes a cached model. vLLM serves one model per process, so Load saves the model into the launch arguments and restarts vLLM on it, restoring the previous model if the new one does not start, and Eject stops vLLM, which frees its GPU memory. Peers and the proxy are told only about the model vLLM is serving.
  • Tool calling for any model: agent clients send tools with tool_choice: "auto" on every request, which vLLM rejects with HTTP 400 unless it was started with a tool-call parser. PAIR now picks the parser from the model itself: it renders the model's chat template with one example tool call and keeps the installed vLLM's parser that reads it back exactly (hf when the tokenizer ships response_template). No list of model families is kept, nothing is downloaded, and no code from the model repository runs. When no parser fits, tool calling stays off and vLLM answers a request that asks for tool calls with its own error.
  • Not offered where it cannot run: vLLM has no Windows build, so on Windows the desktop and TUI no longer show it or offer to install it. Linux and Apple Silicon machines in the same cluster still show it.

Other engines keep their current model and request behavior. Shared code changes in these ways:

  • Engine-agnostic lifecycle fixes (first commit):
    • Stopping an engine also kills helper processes left in its process group.
    • The stop grace is capped at 3 s during shutdown.
    • engine:status and engine:get-installed no longer wait for a running lifecycle operation.
  • New manifest actions: hf_cache, hf_pull, launch_model, and stop_engine, which only the vLLM manifest uses.
  • Engines a machine cannot run (2 commits): engine:get-installed leaves out an engine whose manifest has no block for the host's OS and CPU, where it used to report an empty not-installed entry, and engine:status returns an error for it. The desktop marks an engine missing from a node's listing as unsupported and gives it no row. Today this affects only vLLM.
  • Remote model loads (from review): a remote vLLM load waits on the readiness-sized response budget, like an Ollama load, because it restarts vLLM on the new model. If the caller goes away mid-switch, the previous model is still restored.

Release intent

Changelog title

vLLM engine support

Changelog body

  • vLLM can now be installed and managed from PAIR like the other engines.
  • vLLM can download, load, eject and delete models from the local Hugging Face cache; loading another model restarts vLLM on it, and the previous model is restored if it fails.
  • vLLM picks the right tool-call parser for each model, so agent clients work with Qwen, Granite and other models.
  • vLLM is no longer offered on Windows, where it cannot run.

Bumps

  • services: minor
  • nvpair-cluster-manager: none
  • nvpair-engine-manager: minor
  • nvpair-errors: none
  • nvpair-job-scheduler: minor
  • nvpair-manual-nodes: none
  • nvpair-node-info: none
  • nvpair-node-scanner: minor
  • nvpair-node-settings: none
  • nvpair-proxy: minor
  • nvpair-tui: patch
  • nvpair-ui-broker: minor
  • nvpair-workload-manager: none

Scope

Included: the vLLM engine (manifest, installer, auto-sizing, tool-call parser choice), model download, load, eject and delete, desktop UI, proxy/broker/TUI wiring for the new engine, docs.

Excluded: loading or unloading vLLM models automatically (on a request, or after an idle time), which an earlier revision did through a gateway in the engine manager; it was removed after review because inference traffic belongs to nvpair-proxy and PAIR passes requests through. Also excluded: loaded-node preference in nvpair-proxy (cross-engine, separate change); fixing vLLM memory sizing for some multimodal/quantized models.

This is a large PR, so it is organised as focused commits that can be reviewed one at a time. The groups below are in commit order. The gateway commits in groups 3–6 are undone by the gateway-removal commits in group 10, so a reviewer can skip them and read the final state of services/nvpair-engine-manager instead:

  1. Engine-agnostic groundwork (first 3 commits, each under 500 lines):
    • Stopping slow-starting engines cleanly.
    • The hf_cache manifest action, which lists Hugging Face cache models.
    • The launch_model manifest action, which switches an engine's model through its launch flag.
  2. vLLM engine (next 2 commits): registration across services with the bundled manifest, then its manifest and interop tests.
  3. vLLM features (middle commits): desktop support, auto-sized launch settings, Hugging Face downloads and deletion, the gateway, and idle unload with wake-on-request.
  4. Switching models on request (next 2 commits).
  5. Docs (next commit): brings the vLLM docs in line with the final behaviour: macOS support, advertising every cached model, and the gateway ports.
  6. Tool calling (next 3 commits): the probe that finds a model's tool-call parser, choosing the parser per model in place of the Qwen-only table, and the gateway fallback for models no parser reads.
  7. Engines a machine cannot run (next 2 commits): the engine manager leaves them out of its listing, then the desktop hides them.
  8. Remote model loads (next commit): remote vLLM loads wait for the restart, and the previous model is always restored (from review).
  9. Review follow-ups (next commit): a simpler shard-name pattern and shard-completeness check in the Hugging Face cache scan; deleting the model a stopped vLLM is set to serve now leaves it with no model selected, and a start without a model is refused instead of downloading a default; Delete after Eject works with the gateway; clearer pull wording; one helper for setting and removing launch flags; Qwen3.5-4B as the default model, with a larger install disk check; and launch sizing on unified-memory machines such as DGX Spark.
  10. Gateway removed (last 3 commits, from review): the engine manager no longer proxies vLLM traffic. front.go and its tests, the idle unload, loading on the first request, switching models on request, the tool-removal fallback and the idle-limit actions are removed; vLLM advertises only the model it serves again. The desktop drops the idle state and the idle-limit control. The broker spec, merged from develop meanwhile, now lists the vLLM facade.

Validation

  • go test ./... in services/nvpair-engine-manager, services/nvpair-proxy and the other touched modules, all passing after the gateway removal.
  • npm run lint, typecheck, test:unit, dead-code:check, service-contracts:check in desktop/.
  • go test ./... in services/tests (cross-process, real binaries), run with no other PAIR instance on the machine.
  • node scripts/spdx-headers.mjs.
  • Manual (RTX 6000 Ada): a fresh install from the UI gets vLLM 0.32.0.
  • Manual tool calling after the gateway removal (vLLM 0.32.0, RTX 6000 Ada): Qwen3.5-2B and Granite 4.2 8B load with qwen3_coder and Qwen3-8B with hermes, and each returns a parsed get_weather call, plain and streamed; Nemotron Nano 9B v2 gets no parser, answers plain chat, and a tool request gets vLLM's own 400.
  • Manual after the gateway removal: loading Qwen3-32B (62 GB) on the 48 GB GPU fails and the previous model is restored and answers; after the engine manager restarts, engine:restore-enabled brings vLLM back on the model it was serving, and after a Stop it stays off.
  • Manual (Windows x64, from an installer built from this branch): vLLM is not shown or offered for the Windows node.
  • Manual (real vLLM 0.32.0, isolated home): deleting the configured model of a running vLLM is refused; once stopped it is deleted and --model cleared, a start is refused with no download, and loading another model starts vLLM and answers a chat request.
  • Manual after the gateway removal (real vLLM 0.32.0, isolated home, RTX 6000 Ada): Start loads the Qwen3.5-4B default with vllm serve itself on the engine port, and it answers; Load switches to Qwen2.5-0.5B, which answers, including a tool request with tool_choice: "auto"; only the served model is advertised, and nothing while stopped; a request for another cached model gets vLLM's 404 and changes nothing; Eject stops vLLM and frees the GPU (7.9 GB → 18 MB); Load starts it again; Delete after Eject clears --model, Start is then refused with nothing downloaded, and Load recovers.
  • Manual after the gateway removal (two clustered Linux machines, RTX 6000 Ada and RTX A6000, real vLLM): a remote Load that took 3m5s succeeds and the peer serves the new model; a remote Eject stops vLLM on the peer; a remote Load on the stopped engine starts it again (1m4s); a caller that disconnects 10 s into a remote Load leaves the peer serving its previous model.
  • The probe against the installed vLLM for 8 cached models, after the gateway removal (NVPAIR_LIVE_VLLM_BIN=… NVPAIR_LIVE_VLLM_TOOL_MODELS=… go test -run TestToolParserProbe_FindsTheParserWithTheInstalledVLLM): hermes for Qwen2.5-0.5B, Qwen3-8B and Qwen3-32B, qwen3_coder for Qwen3.5-2B, Qwen3.5-4B and Granite 4.2 8B, none for Nemotron Nano 9B v2 and Qwen2.5-VL-7B NVFP4.

Checklist

  • I have read the Contributing Guidelines.
  • Every commit is signed off (git commit -s), certifying the Developer Certificate of Origin.
  • New or existing tests cover the change.
  • Relevant documentation is updated.
  • I checked the diff, changed filenames, and commit messages for credentials, private data, internal URLs, internal issue identifiers, and generated artifacts.
  • I recorded the validation commands and results above.
  • I declared version bumps in the release-intent block above. services/versions.json is written by automation — do not edit it by hand.

Lifecycle fixes needed by an engine that takes a minute or more to start and
forks helper processes, such as vLLM. None of them is engine-specific.

- engine:status and engine:get-installed no longer wait for a running
  lifecycle operation, so a toggle during a slow start does not time out.
- Shutdown caps the stop grace at 3s so the force kill lands inside the
  parent's window, and a SIGTERM no longer blocks on an idle stdin.
- A stop also kills helper processes left in the engine's process group,
  so a crash mid-load does not orphan a process holding GPU memory.
- An orphaned engine run by an interpreter (a Python package's script) is
  recognised by its command line, so the next launch can stop it.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
…ache

Add an hf_cache manifest action that lists the models in a Hugging Face hub
cache directory. It reads the disk, so it works while the engine is stopped,
which lets an engine that serves one model per process (vLLM) show every model
it could be switched to.

A repository is listed only when its snapshot is a complete, text-model-shaped
download: vision-only and GGUF-only repositories, interrupted downloads and
links whose blob is gone are left out.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
Add a launch_model manifest action for an engine that has no load call because
the model is a launch argument (vLLM). It takes {"model": "<id>"}, writes the
model into the saved launch settings under the declared flag, and restarts a
running engine on it. If the new model does not come up, the previous one is
restored and the error says so. A stopped engine only has its saved launch
changed, and asking for the model already in the launch arguments restarts
nothing.

The action requires editable launch settings and a readiness probe on every
platform, and refuses model ids that could be read as a flag, a path or a
shell fragment.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
Register vllm alongside Ollama, LM Studio and llama.cpp: facade port 8000,
engine port 8001, discovery key "vl". The proxy and broker treat it like any
other managed engine, and the terminal interface gets its model wire.

The engine-manager manifest (Linux amd64/arm64) is detect-first: an existing
vllm is adopted, otherwise a sha256-pinned uv installs vllm 0.32.0 into a
private venv after checking for an NVIDIA driver with CUDA 13+ and enough disk.
vLLM serves one model per process, so its list_models is an hf_cache action,
load_model is a launch_model action, and a new manifest field,
advertised_models: loaded, advertises only the served model to peers and the
proxy, since a request for any other cached model would fail at the engine.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
Check the bundled vLLM manifest's install, launch and model actions, that the
install refuses drivers without CUDA 13, and, across real binaries, that the
broker starts a vLLM proxy and the facade serves the model list and routes
exact model ids.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
Show vLLM next to the other engines: type, display name, links, welcome-screen
default (Linux only), proxy identity and port state, icon and downloaded-model
matcher. The icon is an original lettermark, not the vLLM project logo.

vLLM serves one model per process and has no pull, eject or delete call, so a
new hasPullModel capability hides Add model for it. Its model list is the
Hugging Face cache and Load switches the served model by restarting vLLM; that
load is observed so a failure is attributed to the model row, and the row stays
locked for up to 20 minutes while the engine restarts.

Document the engine across the desktop and top-level docs, tick the roadmap
item, and add the vLLM model store to the data-reset notes.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
Models with a 256K default context need more KV cache than the default
0.5 GPU memory share provides and fail to start. Pass --max-model-len
32768 by default and document it.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
…ts it

vLLM serves one model per process, so loading another restarts the
engine. Show 'starting' instead of 'stopped', keep the model list, and
await the load call so the switch does not look like the engine was
disabled.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
…lling per model

vLLM refuses a context above the model's own limit, reserves 92% of the
GPU by default, and rejects tool_choice auto without a tool-call parser.
Agent clients such as Hermes need a 64K window and tool calling.

Add a manifest auto_launch (hf_context) step that reads the served
model's config.json and the free GPU memory at every start and appends
--max-model-len (64K target, shrunk to fit), YaRN --hf-overrides when the
model's window is shorter, --gpu-memory-utilization sized to the weights
and KV cache, and --enable-auto-tool-choice with the hermes parser for
Qwen families. Arguments the user saved always win, nothing is written
back to saved settings, and tensor/pipeline-parallel launches are left
alone. Drop the fixed 0.5 GPU share and 32K cap from the default launch.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
…for vLLM

vLLM has no pull API, so a model not already cached could only be used by
naming it in the launch arguments. Add an hf_pull action kind for
pull_model: it reads the repo's file list and sizes from Hugging Face,
selects the files vLLM reads, and runs the hf tool from the engine's
virtualenv, reporting progress from the bytes landing in the hub cache.
It needs no running engine and stops when cancelled or stalled.

Add a vLLM engine:catalog source: open, text-generating safetensors repos
of a few publishers (text-generation and image-text-to-text), with
parameter count and size, plus a searchable query like llama.cpp's.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
Enable hasPullModel for vLLM, give it an engine hub, let the model hub
search the upstream catalog for any engine that reports it (a new hubSearch
capability, set for llama.cpp and vLLM), and warm the vLLM catalog with the
others. Update the docs.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
vLLM had no delete, so a cached model could only be removed by hand. Add
an hf_cache delete variant for delete_model: it removes a model's folder
and download locks from the hub cache, needs no running engine, refuses a
path outside the cache, and refuses the model the engine is set to serve
(running or not), failing closed when that cannot be determined.

Enable hasDeleteModel for vLLM in the desktop, with a new
deleteBlockedWhileLoaded capability that disables Delete on the loaded
model. Update the docs.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
vLLM cannot drop its model while running, so Eject (unload_model) is a new
stop_engine action that stops the engine and frees all of its GPU memory.
Load (launch_model) on a stopped engine now saves the choice and starts it,
restoring the previous choice if it does not start. Enable hasEject for
vLLM and keep its model list, Add model and Load available while stopped,
since the list is read from disk.

Delete removed only a model's folder, but recent Hugging Face tooling
keeps large files in a shared hub/blobs store linked from each repo, so the
weights stayed on disk. Delete now also removes shared files no other
model uses, with their sidecars, and reports the bytes freed.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
Add an optional runtime front: PAIR listens on the engine's public port and
runs the engine itself on a loopback port at a fixed offset, forwarding
requests unchanged, streaming responses, cancelling the engine's request
when the client leaves, and answering 503 while the engine is not up. Only
the launch that is spawned is repointed; the launch users see and validate
keeps the public port. A stale engine on the internal port from a crashed
run is reclaimed at the next start.

This is the base for idle unload and wake on request. Only vllm.json
declares a front, and the hooks in shared lifecycle code do nothing
without one; a test checks that every other engine's launch is unchanged.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
An engine behind a gateway now tracks inference activity at the gateway (not
GET/HEAD/OPTIONS, so health checks and the model list do not count, and a
request in flight keeps the engine loaded). After front.idle_timeout_s
(600 s for vLLM, 0 never) with none, the manager stops the engine's process,
freeing the GPU, while the gateway and the ON state stay. The status reports
idle, /health still answers, inference is refused with 503, and the model
it is set to serve is still advertised with nothing marked loaded.

Eject on such an engine now unloads and keeps it ON like the other engines,
and Load of the model it is set to serve loads it again; a failed load
leaves the engine idle. Waking on an incoming request follows.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
A request that needs the model and arrives while the engine is unloaded now
loads it behind the gateway and is answered once it is ready, as Ollama and
llama.cpp do, instead of getting a 503. Requests that arrive together share
one start, a client that leaves does not abandon the load, and Eject followed
by a request reloads it. A failed load answers 503 with the reason and leaves
the engine idle so the next request retries. Health checks, the model list
and other reads never load the model.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
…e proxy

The proxy routes by each node's advertised models and builds its own model
list from the engine's /v1/models. While the engine was unloading or loading,
the inventory briefly lost the model, and the idle gateway refused the model
list, so a request arriving then was answered 'no node advertises the
requested model' and never reached the wake.

Record the served model when the engine goes idle, mark the state idle before
the gateway so no reader sees an engine that is neither loaded nor idle, keep
the engine marked running and idle while it loads (no stopped flicker), and
answer GET /v1/models from the gateway while unloaded or loading.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
…or loading

The health check kept probing /health while the engine loaded again after an
idle unload. The gateway forwards that probe to the engine for the start's
readiness check, so it failed for the whole load and raised 'failed its
health probe', which cleared itself once the load finished. Skip the probe
while the engine is idle or loading, and test that no warning is raised
while one stands.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
…out settable

Start, restart and the restore after PAIR restarts now bring an engine with a
front up idle, with nothing loaded, as Ollama and llama.cpp start a server:
about a second, the engine ON and healthy, the model loaded by the first
request or a Load. The launch is still validated and the ports checked, so a
launch that cannot work is refused at Start. Applying launch settings and
switching or loading the model still load it at once, so a bad setting is
found and rolled back.

Add get_idle_timeout and set_idle_timeout (front_idle) actions: the limit is
read and changed in seconds, 0 for never, applies at once and is saved in the
engine's override file.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
The status of an engine that is on with nothing loaded now carries idle, and
the card reads 'Idle · loads on next request'. For vLLM the model row's expiry
control becomes the idle limit (1 minute to 1 hour, or stays loaded), read
from and sent to the engine manager as seconds through get_idle_timeout and
set_idle_timeout; a limit that is not offered, such as one set by editing the
override file, shows as the nearest one. A new engine-level list marks which
engines use the expiry this way, and a test keeps it and the capability in
step. Update the docs.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
Unload an idle vLLM after five minutes, not ten, and show five minutes as the
limit until the engine manager reports the real one. Update the docs.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
Add a darwin/arm64 platform to the vLLM manifest. The install fetches the
sha256-pinned uv, creates a managed Python 3.12 venv and installs the pinned
vLLM 0.32.0 core and vLLM-Metal wheels, each checked against a digest. It
refuses early on non-arm64, macOS older than 15, or under 10 GB free.

Allow the install on macOS in the desktop engine capabilities, update the
tests that assumed Linux only, and document the new platform.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
vLLM serves one model per process, so a request naming another downloaded
model used to be refused: only the loaded model was advertised.

The gateway in front of vLLM now reads the request's JSON "model" field and,
for a different downloaded model, lets the requests already running finish,
unloads the model, saves --model, loads the new one behind the same gateway,
and answers the held request. Requests that arrive together share one switch.
A model that is not downloaded gets a 404 and nothing changes. A switch that
fails is not rolled back: the request gets a 503 naming the model and the
reason, and vLLM is left unloaded and ON, so the next request loads again.

vLLM now advertises every cached model, including while idle. The gateway's
/v1/models names them all, and loaded_models asks for /v1/models?resident=1 so
only the loaded one is reported as resident.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
- Re-check for a switch under the gateway lock before admitting a request, so
  a request cannot start on a model that is being unloaded or load the old one
  while the switch runs.
- Verify the current model was really unloaded before saving the new one, and
  wait out a load that was only just starting.
- Leave a name that is not a downloaded model (such as a --served-model-name
  alias) to the engine instead of answering 404.
- Read the downloaded-model list once per interval for all requests, keep the
  last good list when a read fails, and bound the model-list body separately
  from the request peek.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
@nvtushark
nvtushark force-pushed the vllm-engine-support branch from 95d8ec6 to 7c0f178 Compare October 28, 2026 17:25
Several docs described vLLM as it was before later commits in this series:

- vLLM also runs on Apple Silicon macOS, not only Linux.
- Every cached model is advertised, and a request for one that is not loaded
  switches vLLM to it; the docs still said only the served model was
  advertised and other requests were refused.
- The PAIR gateway listens on 8001 with vllm serve behind it on loopback
  18001; the docs still put vllm serve on 8001 and said vLLM had no load,
  unload, pull or delete actions.

Also document GET /v1/models?resident=1, and say "vLLM engine" rather than
"vLLM backend" in the services parity table.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
Add a probe that asks the installed vLLM which of its tool-call parsers
reads a model's tool-call format. It runs with vLLM's own Python, renders
the model's chat template with one example tool call, and offers that
text to every registered parser; a parser matches when it extracts
exactly that call. A tokenizer that ships response_template metadata
gets vLLM's hf parser. Nothing is downloaded and no code from the model
repository runs.

Among parsers that read a format equally, the general-purpose ones are
preferred. Results are kept per vLLM binary and snapshot for the life of
the service; a failed probe is tried again on the next start.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
Tool calling was enabled only for the Qwen families, with the hermes
parser, so every other model answered HTTP 400 to agent clients,
which send tools with tool_choice "auto" on every request.

Replace the family table with the probe: a vLLM launch now gets
--enable-auto-tool-choice and the parser that reads the model's own
tool-call format, or no tool flags when none does. A parser named in the
launch arguments is still used as given.

Checked against vLLM 0.32.0: Qwen2.5 and Qwen3 get hermes as before;
Qwen3.5 and Granite 4.2 now get qwen3_coder; Nemotron Nano v2 and
Qwen2.5-VL get none.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
A node's engine manager now leaves out an engine the node cannot run.
Once a node has listed its engines, mark the missing ones unsupported
and give them no row in the engine list, tray, endpoints or first-run
setup. This hides vLLM on Windows, while a Linux node viewed from
Windows still shows it.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
@nvtushark
nvtushark force-pushed the vllm-engine-support branch from 11269f5 to a0eff8f Compare October 29, 2026 14:20

@Noah-Tervalon-Nvidia Noah-Tervalon-Nvidia left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I reviewed everything up through 4e8d2f4 (where you add the front.go). Since I don't really understand the rationale for adding that I skimmed the rest and they all rely on that so going to hold off any further comments.

Comment thread services/installer/linux/INSTALL.md Resolved

// hfDroppedSuffixes are file types a vLLM-served model never reads. Skipping
// them keeps a repo that ships several formats from downloading each of them.
var hfDroppedSuffixes = []string{".gguf", ".onnx", ".h5", ".msgpack", ".ot", ".tflite"}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just asking the question: is this something we want to do? If a user pulls a model they probably expect it to be all there in case they want to go use something else with that model that they assume is all on disk. If they try to run it and parts seem to be missing that might be misleading.

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.

Yes, this is deliberate. A pull downloads what the engine will load, not the whole repository, which is how every engine PAIR manages already behaves: llama.cpp fetches a single GGUF quantization (repo:Q4_K_M), Ollama pulls a single tag, and lms get downloads one variant. vLLM has no download command of its own, so PAIR makes that choice in hfpull.go, and it is the same choice vLLM makes when it downloads a model on first start: the weight format it loads (safetensors when present, otherwise .bin) plus config and tokenizer, skipping original/ checkpoints and other formats. Fetching everything would often multiply the size; Llama 3.3 70B Instruct, for example, carries about 141 GB of safetensors plus about 140 GB of original/ checkpoints.

On using the model with something else: the files sit in the standard Hugging Face cache, which is per file rather than a copy of the repository, so transformers, hf download or another Hugging Face–based tool reuses what is there and fetches only what it is missing. The catalog shows the size of the weights being downloaded, not of the whole repository. I agree it should not be implicit, though, so the action's description and the README now state that it downloads the files vLLM loads and that other formats in the repository are not fetched.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

awesome, thanks for the explanation

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Don't we skip consolidated weights but vLLM includes/prefers them? So it's not quite the same. in some circumstances.

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.

Good catch, thank you. You're right: it wasn't the same set vLLM fetches. Mistral repositories ship every model twice at the same size: Mistral's own format (consolidated*.safetensors with params.json and tekken.json) and the Hugging Face format (model-*.safetensors with config.json). pull_model kept the Hugging Face copy and skipped the consolidated one, but vLLM's default load_format auto loads the consolidated weights whenever a repository has them. So the first Start downloaded the model a second time. With Ministral-3-3B-Instruct-2512, the pull fetched 4.71 GB and Start then added another 4.67 GB.

pull_model now does the reverse: when a repository has consolidated weights, it downloads those and skips the model-*.safetensors shards. The cache listing marks such a model complete based on the same files: the consolidated weights (checked against their index when they're split across several files), params.json or config.json, and the Mistral tokenizer. Repositories without consolidated weights are handled exactly as before. This keeps vLLM in line with the other engines, where a pull fetches only what the engine loads: one Ollama tag, one llama.cpp quantization.

Covered by TestSelectHFFiles_DownloadsTheMistralFormatWeightsVLLMLoads (Mistral Small, Ministral, and consolidated weights split across files) and TestScanHFCache_JudgesAMistralFormatModelOnTheWeightsVLLMLoads. Checked against vLLM 0.32 with Ministral-3-3B: the pull downloads 4.71 GB once, and Start downloads nothing more.

The same test turned up an unrelated problem. vLLM 0.32 accepts mistral_common 1.13, but 1.13 changed a function so that it needs an argument vLLM 0.32 doesn't pass. With the Mistral tool-call parser enabled, every request to a Mistral model returned 400. vLLM 0.33 makes the same call. The installer now pins mistral_common==1.12.0, in a separate commit. I checked it on a fresh install built by the manifest's own script: Ministral and Qwen both answer chat and tool-call requests (auto, required and streamed). Without the pin, every Ministral request returns 400.

if err := os.RemoveAll(repo); err != nil {
return nil, fmt.Errorf("delete %s: %w", id, err)
}
_ = os.RemoveAll(filepath.Join(hubDir, ".locks", name))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is there a reason not to just delete the directory and everything below it (not following sym links)?

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.

That is what happens for the repository itself: its folder is removed with os.RemoveAll, which deletes symlinks without following them. The additional steps cover files that Hugging Face keeps outside that folder. Download locks live in hub/.locks/<repo>, so they are removed separately. Recent Hugging Face tooling also stores the weight files in a store shared by all repositories (hub/blobs/<xx>/<hash>) and leaves only symlinks in the repository's own blobs directory. Deleting the folder alone would therefore leave the weights on disk, so the delete also removes any shared files that no remaining repository links to. Files that another model still uses are kept.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gotcha, makes sense.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Doesn't the implementation use a glob, ignoring Dataset and Space references? How do we prevent deleting one model from removing a shared file some other process needs?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we delegate deletion and shared-blob cleanup to the installed Hugging Face cache API, keeping PAIR’s running-model checks and selection handling? That would preserve Delete’s intended behavior while avoiding our own implementation of cache references and locking.

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.

Agreed, done in dfbbcfb. delete_model now hands the removal to the hf tool that ships beside vllm (the same one pull_model uses), and keeps PAIR's own checks around it.

What PAIR still does, unchanged:

  • validates the model ID and that the path stays inside the cache;
  • refuses the model a running vLLM is serving;
  • for a stopped vLLM, takes the model out of its launch arguments, and puts it back if the delete fails.

What changed:

  • The removal is now one call: hf cache rm model/<org/name> --cache-dir <hub> --yes.
  • huggingface_hub decides which shared-store files go. It reads every repository type through the <hash>.refs manifests and takes the store's <hash>.lock, the same lock its downloads take when they link a file.
  • PAIR's own implementation (sharedHFBlobs, removeUnreferencedHFBlobs, hfBlobTarget and the models--* glob) is removed. PAIR no longer depends on the private layout of HF's shared-blob store.
  • The manifest declares the tool ("hf_cache": {"dir": …, "delete": true, "tool": "hf"}). Validation requires delete and tool together.
  • The tool runs under the action's context, so a timeout or cancel stops it.

This is also how the other engines work in PAIR: the cache's owner does the delete. Ollama uses DELETE /api/delete and llama.cpp uses DELETE /models. For vLLM the cache belongs to Hugging Face, so hf does it.

Two small behaviour changes:

  • The result no longer reports freed bytes, since hf gives that only as text and nothing in PAIR reads it.
  • .locks/<repo> is left in place, as hf cache rm leaves it. These are lock files another process may be holding.

Tests:

  • TestDeleteHFModel_RemovesTheModelWithTheHuggingFaceTool: the exact hf cache rm call, and refused or invalid deletes never reach the tool.
  • TestDeleteHFModel_KeepsTheSelectionWhenTheToolFails: when the tool fails, the model and the stopped engine's selection stay.
  • TestDeleteHFModel_RefusesWithoutTheHuggingFaceTool: with hf missing, nothing is deleted and the selection is not touched.
  • TestHFCacheDeleteActionValidation.

Live check with huggingface_hub 2.3 from the vLLM install: in a scratch cache with real downloads, I added a dataset linking to the same stored file as a model, then deleted the model.

  • The old code removed the shared file and broke the dataset.
  • The new path keeps it: the dataset still reads correctly, and hf frees the file once its last user is gone.

@nvtushark nvtushark Nov 4, 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.

On your first question: you're right on both counts.

  • The glob: the in-use check only globbed models--*, so a file shared with a dataset or Space could be deleted. I reproduced this: deleting a model removed a stored file a dataset still linked to, and broke the dataset.
  • Locking: the check also took no lock, so a download linking the same file at that moment could lose it.

Both are fixed in dfbbcfb by handing the delete to hf cache rm, as you suggested. huggingface_hub reads every repository type's references and takes the store's lock. Details in my reply just above.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we hold one engine lock across the selection change, deletion and rollback? If settings are successfully saved while hf cache rm is running and deletion then fails, this callback restores the entire old launch snapshot and discards that settings edit. (see hfcache.go:354 or so)

Comment thread services/nvpair-engine-manager/front.go Outdated
settings "nvpair-shared/enginesettings"
)

// Front puts a PAIR-owned HTTP gateway on an engine's public port and runs the

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I want to +1 this question. I don't understand the need for another proxy.

…tore the previous model

A remote load used the ordinary 30 s response-header budget for every
engine except Ollama. Loading a vLLM model restarts vLLM on it, which can
take minutes, so the caller gave up, the peer's handler was cancelled mid
start, and the rollback to the previous model ran under the same cancelled
context and failed at once, leaving vLLM down with the new model saved.

Remote vLLM loads now use the readiness-sized client, and the previous
model is restored under a context the caller cannot cancel. A test cancels
a slow switch and checks that the previous model is back and running.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
@nvtushark

nvtushark commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

waitsForEngineReadiness in nvpair-engine-manager/remoteclient.go has a branch for ollama that might also be needed for vllm with regards to remote vLLM loads being canceled too early

Replying to this review point.

@kjlubick Thanks, good catch. Confirmed and fixed in 5c27d65.

For vLLM, load_model restarts the engine on the new model, which can take minutes. A remote load used the ordinary 30 s client (only Ollama loads got the readiness-sized one), so the caller gave up and the peer's handler was cancelled mid-restart. The rollback to the previous model then ran under that same cancelled context, failed straight away, and left vLLM stopped with the new model saved.

The fix:

  • waitsForEngineReadiness now gives vLLM loads the readiness-sized client, like Ollama's.
  • switchLaunchModel restores the previous model under context.WithoutCancel, so a caller that goes away can't block the restore. Shutdown still stops it.
  • New unit test TestLaunchModel_RestoresThePreviousModelWhenTheCallerGivesUp gives up on a slow switch and checks that the previous model is back and running. The remote readiness test gains a vLLM load case.

Verified across two clustered machines with a real vLLM (RTX 6000 Ada → RTX A6000 over LAN mTLS):

  • Before: the remote switch failed at 30 s, and the peer's vLLM stayed stopped with the previous model lost.
  • After: a switch that took 3m24s succeeded. If the caller disconnects mid-switch, the peer restores the previous model by itself and is healthy again within about a minute.

@nvtushark
nvtushark force-pushed the vllm-engine-support branch 2 times, most recently from 167fedb to 13666dc Compare October 30, 2026 14:02
@nvtushark

Copy link
Copy Markdown
Collaborator Author

I reviewed everything up through 4e8d2f4 (where you add the front.go). Since I don't really understand the rationale for adding that I skimmed the rest and they all rely on that so going to hold off any further comments.

(review)

Thank you for the detailed review. I've replied to each of your comments in its own thread, and the resulting changes are in the last commit. On front.go, I've proposed in that thread to take the gateway, with its idle unload and wake-on-request, out of this PR so that vLLM is managed like the other engines. I'll restructure the PR accordingly once you and @kjlubick agree on the direction.

@nvtushark

nvtushark commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

Follow-ups from the review, kept out of this PR

To keep this PR focused, these improvements came out of the review but are deferred. I'll open an issue for each once the PR merges.

  1. Reasoning parser per model. Add --reasoning-parser at start for models that think (Qwen3/3.5, Granite 4.2, Nemotron Nano v2), as is already done for the tool-call parser. (thread)
  2. Let vLLM fit the context. Choose only the memory share and pass --max-model-len auto, removing the KV-cache formula; to be agreed first, since it drops the fixed 64K target and the YaRN extension. (thread)
  3. Show the automatic launch values in the app, not only in the logs. (thread)
  4. Share the GPU-memory reading (nvidia-smi, including unified memory) between node-info and the engine manager through services/shared. (thread)
  5. Split the Hugging Face catalog into shared Hugging Face and per-engine files (catalog_hf.go, catalog_llamacpp.go). (thread)
  6. Evaluate one in-process downloader for all engines that pull from Hugging Face. (thread)

Pending in this PR: removing the vLLM gateway (front.go), as proposed in this thread.

To verify before merge: the Qwen3.5-4B default on Apple Silicon through vLLM-Metal, and launch sizing on a DGX Spark.

Let me know you thoughts / comments on this

- Match Hugging Face shard file names without a leading ^.+ in hfShardRe.
- Drop the redundant shard-count comparison; the loop over shard numbers is
  the real completeness check, and a test now covers shards numbered from zero.
- Allow deleting the model a stopped vLLM is set to serve. The delete takes
  --model out of the saved launch arguments, and an engine with a
  launch_model action refuses to start while its launch names no model,
  rather than letting vLLM download a default of its own. Loading a model
  selects one again. The model a vLLM process serves is still refused; an
  engine ejected behind its gateway is stopped first, so Eject then Delete
  works.
- Say in hfpull.go that the download is done by Hugging Face's hf CLI, and
  state in the pull_model description and README that it fetches the files
  vLLM loads rather than the whole repository.
- Fold setting and removing a launch flag into one helper, dropLaunchFlag.
- Make Qwen/Qwen3.5-4B the default vLLM model in place of Qwen2.5-0.5B, state
  that it needs a GPU with at least 16 GB, and raise the install's free-disk
  check to cover the engine and that model (25 GB on Linux, 20 GB on macOS).
- Size vLLM launches on unified-memory machines such as DGX Spark, where
  nvidia-smi reports [N/A] for GPU memory, from /proc/meminfo, as
  nvpair-node-info already does.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
Resolve two conflicts with develop:
- services/nvpair-engine-manager/README.md: keep develop's shutdown-sweep
  paragraphs and this branch's signal and engine:status paragraphs.
- services/nvpair-ui-broker/enginesettings_test.go: use develop's
  reserveSettingsPort for the harness ports, keeping the fifth port this
  branch added for the vLLM proxy.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
Inference traffic belongs to nvpair-proxy, and the engine manager never
proxies it (spec.md). The gateway in front of vLLM broke that rule and had
PAIR manage vLLM's model use itself, so it is removed, as agreed in review.

vLLM is now handled like a plain server the engine manager starts and
stops: Load saves --model and restarts vLLM, restoring the previous model
if the new one fails, and Eject stops it. Start loads the model straight
away, and only the model being served is advertised, read from vLLM's own
/v1/models.

Removed with it: the idle unload and its limit actions, loading on the
first request, switching models on request, and the fallback that took
tools out of requests to a model with no tool-call parser; vLLM now answers
those requests with its own error. The tool-call parser is still chosen per
model at start.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
The engine manager no longer unloads vLLM when it sits idle, so the card's
"Idle · loads on next request" label and the model row's idle-limit control
go, along with the idle field on the engine status. Eject stops vLLM and
Load starts it on the chosen model.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
The spec merged from develop names the Ollama, LM Studio and llama.cpp
facades only. vLLM's facade is handled like llama.cpp's: a vllm-proxy:
namespace relayed to nvpair-proxy, the default engine forwarder, and a
facade on :8000 or a safe fallback with no compatibility-port gate.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
// Not downloaded yet, so its size and limits are unknown. Only the memory
// share can be bounded sensibly.
if haveGPU && !userUtil && !userKV {
util := math.Floor(math.Min(0.5, float64(free-freeMargin)/float64(total))*100) / 100

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I looked at Qwen/Qwen3.5-4B (the model mentioned in the README). "The model is about 9.3 GB of weights, so the default needs a GPU with at least 16 GB of memory; on a smaller GPU." We set the memory to be half of what we have, so even if the user does have 16 GB of memory, we cap it at 8 and I bet that leads to failures or an unnecessary retry because the model is bigger than that. Could we obtain the model metadata before calculating the first launch’s memory budget? Could we ensure the first launch gets an appropriate budget and cover this empty-cache case with a regression test?

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.

Thanks for checking this against the default model. You're right: with an empty cache we knew nothing about the model, so the first start fell back to a fixed half of the GPU, and on a 16 GiB card that is less than Qwen3.5-4B's 8.7 GiB of weights. The 0.5 was a leftover from the original fixed launch, when the default model was 1 GB.

The first launch now gets a budget sized for the model:

  • Metadata first. When the model is not cached, the start reads its config.json and the sizes of the files a download fetches from the Hugging Face API (10-second limit, HF_TOKEN honoured), the same way vLLM reads config.json from the Hub before it downloads the weights. It is then sized exactly like a cached model: weights, KV cache for the context, and overhead.
  • No fixed share. If the lookup cannot answer, the share is the free GPU memory less a 1 GiB margin, instead of 0.5.
  • At most 32 requests at a time. While testing your case I found a second failure: Qwen3.5-4B is a hybrid model that needs one state block per running request, and vLLM's default of 256 does not fit in 16 GiB ("max_num_seqs (256) exceeds available Mamba cache blocks (167)"). The manifest now declares max_num_seqs: 32 next to the context target, and every start adds --max-num-seqs 32 unless the user sets their own. Measured, 8, 16 and 32 differ by only about 70 MiB.

For Qwen3.5-4B on a 16 GiB GPU, the first start now gets --gpu-memory-utilization 0.87, --max-model-len 12288 and --max-num-seqs 32, instead of a bare 0.50. With the same 13.9 GiB budget on our 48 GiB card, vLLM starts and answers using 12.7 GiB. I don't have a physical 16 GiB GPU to confirm it on.

Regression tests:

  • TestBundledVLLMLaunch_FirstStartOfTheDefaultModelFitsA16GiBGPU runs your empty-cache case through the bundled manifest.
  • TestLookupHFModel_FallsBackWhenHuggingFaceCannotAnswer covers the fallbacks.
  • TestApplyAutoLaunch_LimitsConcurrentRequestsUnlessTheUserSetsALimit covers the request limit.

I also ran your case live through PAIR: with an empty cache, the first start of Qwen3.5-4B was sized from Hugging Face, vLLM downloaded the model and served it, and the next start produced the same arguments from the cache. Tool calls (Qwen3.5, Granite 4.2, Qwen3, Nemotron), the rollback after a failed load, and restore after a restart all still pass with the new request limit.

Fixed in 83dde0b.


// hfDroppedSuffixes are file types a vLLM-served model never reads. Skipping
// them keeps a repo that ships several formats from downloading each of them.
var hfDroppedSuffixes = []string{".gguf", ".onnx", ".h5", ".msgpack", ".ot", ".tflite"}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Don't we skip consolidated weights but vLLM includes/prefers them? So it's not quite the same. in some circumstances.

@@ -0,0 +1,349 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

launchFlagValue seems to return the first thing it finds, not both/all

if err := os.RemoveAll(repo); err != nil {
return nil, fmt.Errorf("delete %s: %w", id, err)
}
_ = os.RemoveAll(filepath.Join(hubDir, ".locks", name))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Doesn't the implementation use a glob, ignoring Dataset and Space references? How do we prevent deleting one model from removing a shared file some other process needs?

// is not what a peer or the proxy may route to, so it is not queried here.
if mf.AdvertisedModels == advertisedModelsLoaded && loadedSpec != nil {
advertiseLoaded[i] = true
listSpec = nil

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we also have remote desktop and TUI model management read the cached inventory? These views currently use the advertised list of loaded models, so ejecting the model empties the management list even though the files remain cached. Users then lose the rows needed to Load or Delete those models.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also, could we distinguish an unavailable remote inventory from a confirmed empty cache here? Stopped vLLM peers advertise no models, so this message can claim nothing is downloaded even when cached files exist, which I think is a bug spotted by QA

if err != nil || json.Unmarshal(raw, &cfg) != nil {
continue
}
return dir, cfg, bytes, true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we select the snapshot matching the revision vLLM will load? For example, the cache could contain:

refs/main → bbb222
snapshots/aaa111/config.json  # older revision: 8K context
snapshots/bbb222/config.json  # current revision: 128K context

os.ReadDir returns entries in filename order, so this selects aaa111 even though cached main points to bbb222. An explicit --revision bbb222 is also ignored here. Automatic configuration can therefore derive context, memory, and tool-parser settings from a different revision than vLLM loads. Could we resolve the intended revision and skip automatic configuration when its snapshot isn’t cached?

if err != nil {
return toolParserProbe{}, err
}
ctx, cancel := context.WithTimeout(context.Background(), toolParserProbeTimeout)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Instead of using context.Background() here, can you pass in a parent context? We want to be able to interrupt this via a higher level cancel or StopAll.

bringUpProcess(ctx, ...)
  → applyAutoLaunch(ctx, ...)
  → planToolCalling(ctx, ...)
  → detectToolParser(ctx, ...)
  → runToolParserProbe(ctx, ...)

return true
}
if len(weights) == 1 {
return true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we restrict the single-file shortcut to unsharded weights? A lone model-00001-of-00004.safetensors currently passes the completeness check even when the other three shards are missing, so an incomplete download can appear ready to load.

Could we also add a test where only model-00001-of-00004.safetensors is present and verify that the model is considered incomplete?

if err := os.RemoveAll(repo); err != nil {
return nil, fmt.Errorf("delete %s: %w", id, err)
}
_ = os.RemoveAll(filepath.Join(hubDir, ".locks", name))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we delegate deletion and shared-blob cleanup to the installed Hugging Face cache API, keeping PAIR’s running-model checks and selection handling? That would preserve Delete’s intended behavior while avoiding our own implementation of cache references and locking.

// not fit in GPU memory, or the id does not exist) the previous model is
// restored, so a failed switch does not leave the engine down. A stopped engine
// only has its saved launch text changed.
func (e *Executor) switchLaunchModel(ctx context.Context, engine string, spec *ActionLaunchModel, params json.RawMessage) (json.RawMessage, error) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we extract the shared launch-change machinery behind ConfigureLaunch and reuse it for model switching and selection clearing? This could remove repeated snapshot/configure/start/restore sequences and hold one engine lock across the operation, while preserving each operation’s existing failure policy.

@Noah-Tervalon-Nvidia

Copy link
Copy Markdown
Collaborator

this looks good to me once we address Kaylee's comments

On a fresh install the default model is not cached yet, so the first
start knew nothing about it and claimed a fixed half of the GPU. On a
16 GiB card that is less than Qwen3.5-4B's 8.7 GiB of weights, and the
start failed.

- When the model is not cached, read its config.json and the sizes of
  the files a download fetches from Hugging Face (10 s limit, HF_TOKEN
  honoured, skipped with HF_HUB_OFFLINE) and size the launch exactly as
  for a cached model. vLLM reads config.json from the Hub the same way
  before it downloads the weights.
- When that lookup cannot answer, give the free GPU memory less the
  1 GiB margin instead of a fixed 0.5.
- Add auto_launch.max_num_seqs to the manifest (32 for vLLM) and pass
  --max-num-seqs unless the user set it. vLLM's default of 256 needs one
  state block per request for a hybrid model such as Qwen3.5-4B, and
  256 of them do not fit in 16 GiB.
- launchFlagValue now returns the last occurrence of a repeated flag, as
  vLLM's argument parser does, and stops at a "--" terminator.

Tests cover the empty-cache 16 GiB case through the bundled manifest,
the lookup and its fallbacks, the request limit, and repeated flags.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
…mat vLLM loads

A Mistral repository ships each model twice at the same size: Mistral's own
consolidated*.safetensors with params.json and tekken.json, and the Hugging
Face model-*.safetensors with config.json. pull_model kept the Hugging Face
copy and dropped the consolidated file, but vLLM's default load_format auto
prefers the consolidated weights whenever the repository has them, so the
first Start downloaded a second full copy (Ministral-3-3B: 4.71 GB pulled,
then 4.67 GB more on Start).

When a repository has consolidated weights, pull_model now downloads those
and skips the model-*.safetensors shards, and the cache listing judges such a
model complete on the consolidated weights (and their index when sharded),
params.json or config.json, and the Mistral tokenizer.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
vLLM 0.32 requires only mistral_common>=1.11.6, so a new install resolves
1.13. That release made the force_tool_names argument of
GrammarFactory.get_lark_from_jinja required, and vLLM 0.32 does not pass it.
With the Mistral tool-call parser that PAIR enables for Mistral models, every
chat request to such a model returned HTTP 400.

Pin mistral_common==1.12.0 on every platform. vLLM 0.33 makes the same call,
so the pin changes only with an upgrade to a vLLM release that itself
requires mistral_common 1.13.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
delete_model removed a model's folder from the Hugging Face cache and then
cleaned up the cache-wide shared blob store itself. Its in-use check globbed
only models--* folders, so a stored file a dataset or Space still linked to
was deleted with the model, and it took none of the store's locks, so a
download linking the same file at that moment could lose it.

The removal now runs `hf cache rm model/<org/name> --cache-dir <hub> --yes`
with the hf tool beside vllm, the one pull_model already uses. huggingface_hub
reads every repository type's references and takes the store's lock. PAIR
keeps its own checks around it: the id and path checks, refusing the model a
running vLLM serves, and taking a stopped vLLM's model out of its launch
arguments, which is put back if the tool fails. The tool runs under the
action's context.

The manifest names the tool (hf_cache.tool), and validation requires delete
and tool together. The result no longer reports freed bytes, which nothing
read, and .locks/<repo> is left in place as hf leaves it.

Signed-off-by: Tushar Khinvasara <tkhinvasara@nvidia.com>
return nil, fmt.Errorf("%q is not a valid model id (want a Hugging Face id such as org/name)", request.Model)
}

current, err := e.LaunchSettings(engine)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA reports that loading nvidia/Llama-3.1-8B-Instruct-FP8 on Apple Silicon fails before readiness because vLLM-Metal rejects the fp8_e4m3 KV-cache configuration, and PAIR displays the traceback. Could you verify this with the bundled Metal version and provide either a supported configuration or clear compatibility guidance? Could we cover this case with a regression test, including preservation of the previous working model? Any catalog restriction should target the Mac and leave DGX Spark unaffected.

if hasParser {
return []string{"--enable-auto-tool-choice"}, userParser
}
snapshot, _, _, cached := findHFSnapshot(hubDir, model)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we prepare the tokenizer/template before selecting the first launch’s tool parser? For an uncached model, this returns without the parser flags, so vLLM downloads and serves the model but rejects agent tool requests until a restart.

if err := os.RemoveAll(repo); err != nil {
return nil, fmt.Errorf("delete %s: %w", id, err)
}
_ = os.RemoveAll(filepath.Join(hubDir, ".locks", name))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we hold one engine lock across the selection change, deletion and rollback? If settings are successfully saved while hf cache rm is running and deletion then fails, this callback restores the entire old launch snapshot and discards that settings edit. (see hfcache.go:354 or so)

"env": { "HF_HOME": "{models_dir}", "VLLM_USE_FLASHINFER_SAMPLER": "0" },
"bind": "127.0.0.1",
"port": 8001,
"ready": { "http": "http://127.0.0.1:{port}/health", "status": 200, "timeout_s": 480 },

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we keep desktop Start pending for this readiness window? Its marker currently expires after 90 seconds while vLLM allows 480 seconds, so a still-loading engine appears stopped and clicking its toggle sends another Start instead of Stop.

}

func TestLaunchFlagValue_ReadsTheValueVLLMWillUse(t *testing.T) {
for _, tc := range []struct {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please follow the repository style for table based tests, using a test function. Please also use the testify assert/require helpers. Here and in other go tests.

// A running engine is restarted, and if the new model does not come up (it does
// not fit in GPU memory, or the id does not exist) the previous model is
// restored, so a failed switch does not leave the engine down. A stopped engine
// only has its saved launch text changed.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we update the comments to reflect that Load starts a stopped vLLM? This comment says only the saved arguments change, and empty-handlers.ts:268 still says all Load operations require a running engine.

return err
}
if err := apply(ctx, text, false); err != nil {
if !current.Running {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we remove the unreachable checks left after the rework? The stopped-engine branch above always returns, so current.Running must be true here.

// A model switch restarts the engine, so it reports stopped on the
// way. The list of models does not change, and clearing it made the
// card empty until the engine was back.
if (getModularBridgeState().isLocalModelSwitching(engineType)) return

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This appears to check for vLLM but the above condition excludes it, I think.

func (c *catalogService) searchCache(key, label string, fetch func(context.Context) ([]CatalogModel, error)) *listingCache {
c.searchMu.Lock()
defer c.searchMu.Unlock()
search, ok := c.llamaSearches[key]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we give llamaSearches and llamaCPPSearchCacheLimit engine-neutral names now that this cache also serves vLLM?

modelOpsWhenStopped: false,
// The router removes cached models in-process via DELETE /models.
hasDeleteModel: true,
hasPullModel: true,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you explain why we need hasPullModel when it's true for all supported engines?


stdinRead, stdinWrite := io.Pipe() // never written to, never closed
t.Cleanup(func() { _ = stdinWrite.Close() })
var out syncBuffer

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Consider io.Discard to simplify this test a lot.

}
resp, err := e.client.Do(req)
if err != nil {
var urlErr *url.Error

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we reuse withoutURL?

jobs = append(jobs, job{v, p})
}
}
batches := make([][]hfModelRow, len(jobs))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we share some of this logic with fetchLlamaCPP

return q
}

func normalizeVLLMRows(rows []hfModelRow) []CatalogModel {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we deduplicate this with normalizeLLamaCPPRows ?

return // already exited
default:
}
pid := mp.cmd.Process.Pid

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

defer killGroupRemnants(pid) instead of three calls to it?

// engine's output pipes, so the stop sees the main process end and nothing
// else would ever notice the helper.
script := `trap 'exit 0' TERM; (trap '' TERM; exec sleep 300 >/dev/null 2>&1) & echo $!; while true; do sleep 0.1; done`
var mu sync.Mutex

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we need this mutex? It's not clear to me what the mutex is protecting.

const flush = () => new Promise(resolve => setTimeout(resolve, 0))

it('awaits load_model and shows the engine as starting, not stopped, until it finishes', async () => {
let finish: (value: undefined) => void = () => undefined

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Promise.withResolvers<undefined>() might make this a bit more concise

errs := make([]error, len(jobs))
var wg sync.WaitGroup
for i, j := range jobs {
wg.Add(1)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

WaitGroups now have a .Go() helper to make this more concise.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants