Repository navigation
Conversation
Add a swappable model registry to so the model behind a stable route name can change without reconfiguring clients. - POST /v1/models/load swaps the loaded model; GET /v1/models lists the registry with loaded flags; generation requests are validated: unknown model is a 404, registered-but-not-loaded is a 409 pointing at the load endpoint instead of silently serving whatever is resident. - CLI: --models PATH[=ID] registers swappable containers (repeatable), --keep-previous keeps a replaced model resident instead of unloading it. - Swap opens the new container before closing the old one, so a failed load leaves the previous model serving; swap happens under the current engine's lock so a generation in flight finishes first. - Request log lines now name the model: the one that served a success, the one refused on a 409/404, the one a load switched to. New --no-log-requests silences request logging. - Tests over real sockets cover registry listing, swap, keep-previous, validation statuses, and the model names in the log; docs updated.
…no swap A request used to read srv.engine before taking any lock, and read the other per-model facts (chat_format, model_info, stop_tokens, default_thinking, markers, chat_error) one attribute at a time. Two interleavings broke: a request granted the old engine's lock only after a swap had closed it called state_reset()/generate() on a dead ctx and answered 500; and a request could see the new engine with the old chat format — building an XTML prompt for a container that speaks chat.json — because _detect() re-bound the attributes one at a time under a lock the reader had not taken. The per-model facts are now one immutable ModelSlot (engine, model_id, model_info, markers, chat_format, chat_error, stop_tokens, default_thinking, plus the parser factory), built whole by ModelSlot.detect() and published in a single assignment. Each request takes current_slot() once, locks that snapshot's engine, and re-checks with check_engine() that the slot is still current before touching the engine — answering 409 model_switched with the new current model id otherwise. The same discipline covers /v1/completions, and the streaming/blocking tails report stats and model from the slot rather than re-reading the moving server mid-generation.
A swap opens the new container before closing the old one — the order that makes a failed open leave the server serving what it was serving — so two contexts are resident at once. docs/SERVE.md asked the operator to size --budget so that moment fits, and nothing checked it. With the default --budget 0 the moment was worse than "the sum of the two": each context sizes itself to up to 3/4 of waste_usable_ram (waste.h, waste_cfg.ram_budget_bytes), so a swap ran at ~1.5x what the process may use — a paging run, not a slow one. With --keep-previous the total is not two but every model ever loaded, which no startup check can price at all. - --models now requires an explicit --budget, and the server refuses to start unless 2 x budget fits, naming the largest budget that does; the same lines print under the startup banner and under --plan, which is where a budget gets chosen. - The resident set is counted at every load — each engine's declared budget, or the floor plus expert cache waste_memory_used reports for one that chose its own — and a load that would not fit is refused with 507 (insufficient_memory) *before* the container is opened: nothing is closed, nothing is half-loaded, the previous model keeps serving, and the message says what it needed next to what was already held. 507 rather than 503 because asking again cannot help. - A slot move to a model that is already resident is never refused: it allocates nothing. Evicting a resident model to make room is not done — changing what is resident behind a client's back is the failure --keep-previous exists to prevent. serve/api.py gains human_bytes, shared by the banner and the refusal, so the two cannot quote different figures for the same machine. A platform that will not report its RAM is warned about, not refused: a limit nobody can measure is not a limit. Tests: 9 new server tests (144, from 135) and tests/serve/test_main.py with 12 of its own — the arithmetic needs no container, no libwaste and no particular machine. Checked end to end against real containers too: the banner, /v1/models, a --keep-previous swap, and a 507 from the third resident at a declared budget of 24G on a 64G machine.
The registry made check_model_request strict for every deployment: with no --models the registry holds only the loaded model, so a request naming anything else was a 404 — breaking every client that sends a fixed model name it cannot easily change, including serve --help's own example of "model": "waste". The server's id defaults to the container's file name (k3.waste -> k3), so even the correct name was one most clients did not know. Validation now applies only when the registry names a swappable set — len(registry) > 1, which is what --models gives — and a single-container server answers any name with the one model it has, as it did before the registry existed. Strict validation is unchanged where it is wanted: a 404 for a name the registry does not know, a 409 for one it knows but has not loaded, on both chat completions and raw completions, with the model-before-shape ordering the OpenAI API uses. docs/SERVE.md no longer describes the 404 as the default behaviour, and the CHANGELOG carries the break-and-restore under Unreleased. Tests: the no-registry case is covered where it lives now (a foreign name served, shape validation first), the registry case keeps its 404/409 and ordering tests, and /v1/completions gets the coverage chat had. Suite is 149 server tests, all green.
With --keep-previous a model the server still holds an open waste_ctx
for was listed loaded:false on GET /v1/models and /v1/models/{id} -
indistinguishable from a container never opened - while POST
/v1/models/load counted it in its resident set and check_model_request
called it 'not loaded'. The flag is residency: mid in engines, the same
set the load response reports. The 409 for a resident-but-idle model
names residency too; the model_not_loaded type and status are unchanged.
The waste shape still travels only on the current entry - the per-model
facts move with the current slot and are re-derived when a swap makes a
resident model current again.
Refusing --usage when --models is provided rejects this invalid configuration upfront, explaining clearly that --usage is container-specific and cannot be shared across multiple models. At the same time, removing "usage_path": args.usage from engine_kwargs in serve/__main__.py ensures that swap operations always let each container find its own hotlist (the default None behavior), preventing any accidental propagation of a single hotlist to swappable models.
To ensure model swaps report container capabilities and any chat formatting limitations to the operator, we log a line to `sys.stderr` on each swap in `ChatServer.load_model`, reporting the model switched to and its `chat_error` when set.
the load endpoint may be given a pathname instead of (or alongside) a known id. The server derives the model's id from the path — the same convention --models PATH[=ID] already uses on the CLI (file name minus .waste, or an explicit id) — adds it to the registry
serve: fix lock order inversion
register_path edits the registry before load_model prices anything, so a load that check_room refuses with 507 left its id behind. The id then loaded by name alone, with no path in the body -- bypassing the --auto-register gate on the one request that never passed it. Roll back exactly the id this request introduced (known_before diff), so a refused re-load of an operator's --models entry never deregisters it. Full serve suite: 406 tests, 0 failures.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rolls back the id a refused
--auto-registerload leaves in the registry — the defect reported in #74 (comment of 2026-09-25), whereregister_patheditsself.registrybeforeload_modelprices anything and nothing unwinds it. Branch isbc91574(PR #74 head) plus this fix, per "By all means, open the PR."The bug
A load that
check_roomrefuses with 507 still registers the path. The leaked id then loads by name alone, with nopathin the body — it never passes the--auto-registercheck a second time, because the gate is consulted on the request that failed, not the one that succeeds:One refused request permanently widens the id namespace the server accepts. That matters precisely because
--auto-registeris the gate in front of "open an arbitrary file on this machine".The fix
known_beforediff keeps the rollback honest: only an id this request introduced is rolled back, so re-loading an operator's--modelsentry never deregisters it on a 507. Same rollback discipline the open-before-close order already gives the engines, extended to the registry.BaseExceptionso an engine crash mid-open cannot leak the id either.Tests
TestAutoRegisterRefusedRollback(tests/serve/test_server.py, 4 tests; budget 10 × 2 > usable 15 so every swap is refused,FakeEngineharness):--modelsentry survives a refused path load of the same idFull suite on this branch:
Ran 406 tests ... OK (skipped=83)— same invocation astests/run.sh, nothing else changed.Notes