Swap the resident model without restarting mavend #68

Closed
claude wants to merge 1 commits from overnight/model-swap into overnight/web-crawler
Contributor

What

Swapping the resident model no longer needs a restart. internal/phraser/swap.go adds
LLMPhraser.Swap, reachable only through two new IPC methods (swap_model,
model_status) and a new /models page in mavweb.

The capability does not exist unless phraser.swap_models lists the gguf paths the
operator allows. Absolute paths only, exact match, validated at startup.

Why

docs/plans/05-model-swap.md / Vikunja #250. With a CPT'd Qwen3-1.7B landing soon, the
A/B loop is "edit deploy/mavend.json, restart the daemon, lose the socket, the wake
loop and any in-flight turn". This makes it one authenticated click.

Safety properties, as code not prose

  • Never two models resident. Kill-then-load: the old llama-server is killed and
    reaped before the new one launches. One 1.7B fits the Vega iGPU with n_gpu_layers: 99; a blue/green overlap would OOM the laptop, so it is not offered.
    swap_test.go asserts maxLive <= 1 across a swap.
  • Atomic per turn. quiesce drains in-flight requests (they finish on the old
    model) and acquire then refuses arrivals with ErrSwapping until the new server has
    answered /v1/models. No turn is served half-swapped; refused turns degrade to the
    classifier cascade, which is exactly what it is there for.
  • A failed load rolls back. Launch or probe failure reloads the previous model and
    returns RolledBack: true alongside the error, so the UI can say "she is still
    answering, with the old model". If the rollback also fails, the daemon logs that no
    model is loaded and keeps serving from the classifier instead of pretending.
  • Single-flight. swapMu serialises swaps; a second concurrent one is refused.
  • Not reachable by Maven. MethodSwapModel is AuthStepUp in internal/auth
    (same rung as mutating the tool allowlist), so a surface that cannot carry a passkey
    gesture — voice, chat — is refused on shape alone. POST /models goes through the
    same stepUpOK as POST /tools. Nothing calls Swap on a timer and no act or intent
    reaches it.
  • No arbitrary file load. The request carries a path that becomes llama-server's
    -m, so anything looser than an exact allowlist match would be a file-read primitive.
    The configured model_path is always allowed back to, listed or not — the way out of
    a bad swap must not depend on remembering to allowlist what you are already running.

Also

llm.Client now guards its base URL behind an RWMutex and gains
SetBaseURL/BaseURL; LLMPhraser.OnSwap re-points every holder (LLM router, replier,
mail extractor, memory evaluator) so they follow the new port. Without this the daemon
would fall to the classifier permanently after the first swap. NewLLMPhraserAt (a
borrowed server, tests and remote setups) leaves the launcher nil and Swap refuses
with ErrSwapNotOwned — we do not kill a server we did not start.

Deliberately not done

The plan's remote-backend half (phraser.mode / remote_url, swapping between local
and a llama-server on another box) is not here. It is a different problem — a model on
another host is not drained, killed or rolled back by this code — and folding it in
would have made the drain semantics guesswork. ErrSwapNotOwned is the honest refusal
in the meantime.

Verified

  • make build — all 10 binaries, exit 0.
  • make test — full -race suite, exit 0.
  • New tests: internal/phraser/swap_test.go (8 cases, against an injected fake fleet of
    httptest llama-servers, so no gguf and no GPU needed) covers repointing holders,
    rollback on launch failure, rollback on probe failure, rollback-of-the-rollback
    degrading cleanly, drain-then-refuse, ErrSwapNotOwned, same-model no-op, empty path.
    Plus cmd/mavweb/models_test.go (7 cases, incl. the step-up denial reaching nothing),
    internal/ipc off-unless-wired, internal/auth authority + voice refusal,
    internal/config allowlist parsing and relative-path rejection, internal/llm
    re-pointing.
  • Not exercised against a real gguf on the iGPU — that is the QA step on homesrv.

Vikunja #250

## What Swapping the resident model no longer needs a restart. `internal/phraser/swap.go` adds `LLMPhraser.Swap`, reachable only through two new IPC methods (`swap_model`, `model_status`) and a new `/models` page in mavweb. The capability does not exist unless `phraser.swap_models` lists the gguf paths the operator allows. Absolute paths only, exact match, validated at startup. ## Why `docs/plans/05-model-swap.md` / Vikunja #250. With a CPT'd Qwen3-1.7B landing soon, the A/B loop is "edit `deploy/mavend.json`, restart the daemon, lose the socket, the wake loop and any in-flight turn". This makes it one authenticated click. ## Safety properties, as code not prose - **Never two models resident.** Kill-then-load: the old llama-server is killed and reaped before the new one launches. One 1.7B fits the Vega iGPU with `n_gpu_layers: 99`; a blue/green overlap would OOM the laptop, so it is not offered. `swap_test.go` asserts `maxLive <= 1` across a swap. - **Atomic per turn.** `quiesce` drains in-flight requests (they finish on the old model) and `acquire` then refuses arrivals with `ErrSwapping` until the new server has answered `/v1/models`. No turn is served half-swapped; refused turns degrade to the classifier cascade, which is exactly what it is there for. - **A failed load rolls back.** Launch or probe failure reloads the previous model and returns `RolledBack: true` alongside the error, so the UI can say "she is still answering, with the old model". If the rollback also fails, the daemon logs that no model is loaded and keeps serving from the classifier instead of pretending. - **Single-flight.** `swapMu` serialises swaps; a second concurrent one is refused. - **Not reachable by Maven.** `MethodSwapModel` is `AuthStepUp` in `internal/auth` (same rung as mutating the tool allowlist), so a surface that cannot carry a passkey gesture — voice, chat — is refused on shape alone. `POST /models` goes through the same `stepUpOK` as `POST /tools`. Nothing calls `Swap` on a timer and no act or intent reaches it. - **No arbitrary file load.** The request carries a path that becomes llama-server's `-m`, so anything looser than an exact allowlist match would be a file-read primitive. The configured `model_path` is always allowed back to, listed or not — the way out of a bad swap must not depend on remembering to allowlist what you are already running. ## Also `llm.Client` now guards its base URL behind an RWMutex and gains `SetBaseURL`/`BaseURL`; `LLMPhraser.OnSwap` re-points every holder (LLM router, replier, mail extractor, memory evaluator) so they follow the new port. Without this the daemon would fall to the classifier permanently after the first swap. `NewLLMPhraserAt` (a borrowed server, tests and remote setups) leaves the launcher nil and `Swap` refuses with `ErrSwapNotOwned` — we do not kill a server we did not start. ## Deliberately not done The plan's remote-backend half (`phraser.mode` / `remote_url`, swapping between local and a llama-server on another box) is not here. It is a different problem — a model on another host is not drained, killed or rolled back by this code — and folding it in would have made the drain semantics guesswork. `ErrSwapNotOwned` is the honest refusal in the meantime. ## Verified - `make build` — all 10 binaries, exit 0. - `make test` — full `-race` suite, exit 0. - New tests: `internal/phraser/swap_test.go` (8 cases, against an injected fake fleet of httptest llama-servers, so no gguf and no GPU needed) covers repointing holders, rollback on launch failure, rollback on probe failure, rollback-of-the-rollback degrading cleanly, drain-then-refuse, `ErrSwapNotOwned`, same-model no-op, empty path. Plus `cmd/mavweb/models_test.go` (7 cases, incl. the step-up denial reaching nothing), `internal/ipc` off-unless-wired, `internal/auth` authority + voice refusal, `internal/config` allowlist parsing and relative-path rejection, `internal/llm` re-pointing. - Not exercised against a real gguf on the iGPU — that is the QA step on homesrv. Vikunja #250
claude added 1 commit 2026-08-01 01:59:37 +02:00
Loading a different gguf was a one-line edit to phraser.model_path plus a
restart. It is now an owner-triggered IPC call, off unless configured.

internal/phraser/swap.go holds the safety properties as code:

  - Never two models resident. The old llama-server is killed and reaped
    before the new one is launched. One 1.7B fits the Vega iGPU; a
    blue/green overlap would OOM the box, so it is not offered.
  - Atomic from a turn's point of view. Swap drains the in-flight turns
    (they finish on the old model), then refuses arrivals with ErrSwapping
    until the new server has answered /v1/models. No turn ever sees half a
    swap; refused turns fall back to the classifier cascade.
  - A failed load rolls back. If the new model does not start or does not
    probe, the previous one is reloaded and the call returns RolledBack
    with the error. If the rollback also fails the daemon says so and
    degrades to the classifier rather than pretending to serve.

Holders of the completion client are re-pointed, not rebuilt: llm.Client
guards its base URL and LLMPhraser.OnSwap re-points it, so the router, the
replier, the mail extractor and the memory evaluator follow the new port
without knowing a swap happened.

Reach is deliberately narrow. phraser.swap_models is an exact-match
allowlist of absolute paths a human wrote, rejected at startup otherwise,
so "swap the model" can never mean "load any file on my disk"; the running
model is always swappable back to. MethodSwapModel is AuthStepUp, the same
rung as mutating the tool allowlist, and /models gates POST through the
same stepUpOK the tools page uses. Nothing calls Swap on a timer and no
act, intent or utterance reaches it.

Vikunja #250
claude reviewed 2026-08-01 11:34:44 +02:00
claude left a comment
Author
Contributor

Kill-then-load rather than blue/green is the right call for this box, and the file comment argues it instead of asserting it. One 1.7B fully offloaded to a Vega iGPU leaves no room for a second copy. The "keep the old one warm" design would OOM the laptop. The new server is probed for its own model identity before it is published, and that identity is reported instead of an echo of the request. A mislabelled gguf shows up on the page, not in a week of bad replies. launch staying nil in NewLLMPhraserAt so Swap refuses a server it did not start is the detail that keeps the eval harness safe. The allowlist is exact paths a human typed. cfg.Phraser.ModelPath is always included, so the way back never depends on remembering to list it.

Four things.

1. Total failure reads as success-with-fallback. Swap sets RolledBack: true on both failure paths, including the one where the rollback itself failed and p.be is nil. handleModels branches on res.RolledBack before anything else and renders:

swap failed, rolled back to — she is still answering, with the old model

res.Model is empty there, and she is not answering. Every phrasing path is on its template fallback and routing is on the classifier. This is the exact state the operator most needs to see, and the page tells him the opposite. Give the total failure its own flag, or leave RolledBack false when nothing was rolled back to.

2. The drain counts the phraser's own calls and nothing else. acquire is called from chatWithSystem and chatWithMessages. The LLM router, the replier, the mail extractor and the memory evaluator all hold an *llm.Client built by llmClientFor and go straight to Complete. None of them touch inflight and none of them see swapping. So quiesce can return with a count of zero while the router is mid-generation, and old.Close() kills the server under it. Two documented properties fail:

  • "An in-flight turn finishes on the old model" is true only for phrasing, not for routing.
  • "No turn ever gets half of one model and half of another" is false. A turn routed by the old model, killed before it phrases, then phrased by the new one is exactly that. It degrades rather than breaks, which is why it will not be noticed.

The gate belongs where the base URL is read. llm.Client already goes through BaseURL() under a lock, so an acquire-shaped hook there would cover every holder.

3. A swap freezes the whole web UI. No deadline anywhere will break it. Client.call holds c.mu for the entire roundtrip, serveConn reads one frame at a time, and neither side sets a read deadline. The server-side ctx is context.Background(), so a browser giving up does not shorten anything. Worst case adds up: 90s drain, 60s launch, 30s probe, then a rollback of another 60s and 30s. For roughly four and a half minutes /dash, /history, /notifications and every other mavweb page block on the same client mutex. No timeout frees them. The swap needs its own connection, or the response needs to come back before the load finishes.

4. POST /models is missing from the route table in cmd/mavweb/main.go. That table makes each new route's gate a deliberate decision. This is the second route added since it landed that skips it. The gate itself is right and matches /tools for the right reason. Add the row.

Smaller notes:

  • Two comments say llama-server is "relaunched on a fresh port". startLlamaProc passes extractPort(cfg.Listen), the same fixed port every time. The new server binds the port the killed one just released, and the base URL does not change. SetBaseURL is then a no-op on every normal swap, and the observer mechanism is only load-bearing if the port ever does move. Say which one is intended.
  • The total-failure path never touches p.live, so LiveModel() keeps reporting the old model path. /models then shows a file row naming a gguf that is not loaded next to a model row saying unknown. Clear p.live when there is no backend.
  • The total-failure log says routing is on the classifier "until the daemon is restarted". Not so: Swap is still wired and a second attempt can recover without a restart. That is the more useful instruction to leave in the log.
  • handleModels reads n_ctx off the form, and the template renders no such input. Reachable only by hand-crafting a POST. CLAUDE.md pins n_ctx at 4096 because the resident model is a Thinking variant. A non-Thinking gguf inherits a window sized for reasoning tokens it never emits. If the field is meant to be usable, render it next to each allowlisted path. NGpuLayers has the same shape.
  • Nothing persists which model was swapped in, so a restart silently returns to phraser.model_path. That is defensible as policy, and it is invisible on the page. One line saying so would close it. It matters more with PR 69 next in the stack. An update restarts the daemon, so it undoes any swap without saying it did.
Kill-then-load rather than blue/green is the right call for this box, and the file comment argues it instead of asserting it. One 1.7B fully offloaded to a Vega iGPU leaves no room for a second copy. The "keep the old one warm" design would OOM the laptop. The new server is probed for its own model identity before it is published, and that identity is reported instead of an echo of the request. A mislabelled gguf shows up on the page, not in a week of bad replies. `launch` staying nil in `NewLLMPhraserAt` so `Swap` refuses a server it did not start is the detail that keeps the eval harness safe. The allowlist is exact paths a human typed. `cfg.Phraser.ModelPath` is always included, so the way back never depends on remembering to list it. Four things. **1. Total failure reads as success-with-fallback.** `Swap` sets `RolledBack: true` on both failure paths, including the one where the rollback itself failed and `p.be` is nil. `handleModels` branches on `res.RolledBack` before anything else and renders: > swap failed, rolled back to — she is still answering, with the old model `res.Model` is empty there, and she is not answering. Every phrasing path is on its template fallback and routing is on the classifier. This is the exact state the operator most needs to see, and the page tells him the opposite. Give the total failure its own flag, or leave `RolledBack` false when nothing was rolled back to. **2. The drain counts the phraser's own calls and nothing else.** `acquire` is called from `chatWithSystem` and `chatWithMessages`. The LLM router, the replier, the mail extractor and the memory evaluator all hold an `*llm.Client` built by `llmClientFor` and go straight to `Complete`. None of them touch `inflight` and none of them see `swapping`. So `quiesce` can return with a count of zero while the router is mid-generation, and `old.Close()` kills the server under it. Two documented properties fail: - "An in-flight turn finishes on the old model" is true only for phrasing, not for routing. - "No turn ever gets half of one model and half of another" is false. A turn routed by the old model, killed before it phrases, then phrased by the new one is exactly that. It degrades rather than breaks, which is why it will not be noticed. The gate belongs where the base URL is read. `llm.Client` already goes through `BaseURL()` under a lock, so an `acquire`-shaped hook there would cover every holder. **3. A swap freezes the whole web UI.** No deadline anywhere will break it. `Client.call` holds `c.mu` for the entire roundtrip, `serveConn` reads one frame at a time, and neither side sets a read deadline. The server-side ctx is `context.Background()`, so a browser giving up does not shorten anything. Worst case adds up: 90s drain, 60s launch, 30s probe, then a rollback of another 60s and 30s. For roughly four and a half minutes `/dash`, `/history`, `/notifications` and every other mavweb page block on the same client mutex. No timeout frees them. The swap needs its own connection, or the response needs to come back before the load finishes. **4. `POST /models` is missing from the route table in `cmd/mavweb/main.go`.** That table makes each new route's gate a deliberate decision. This is the second route added since it landed that skips it. The gate itself is right and matches `/tools` for the right reason. Add the row. Smaller notes: - Two comments say llama-server is "relaunched on a fresh port". `startLlamaProc` passes `extractPort(cfg.Listen)`, the same fixed port every time. The new server binds the port the killed one just released, and the base URL does not change. `SetBaseURL` is then a no-op on every normal swap, and the observer mechanism is only load-bearing if the port ever does move. Say which one is intended. - The total-failure path never touches `p.live`, so `LiveModel()` keeps reporting the old model path. `/models` then shows a `file` row naming a gguf that is not loaded next to a `model` row saying unknown. Clear `p.live` when there is no backend. - The total-failure log says routing is on the classifier "until the daemon is restarted". Not so: `Swap` is still wired and a second attempt can recover without a restart. That is the more useful instruction to leave in the log. - `handleModels` reads `n_ctx` off the form, and the template renders no such input. Reachable only by hand-crafting a POST. CLAUDE.md pins `n_ctx` at 4096 because the resident model is a Thinking variant. A non-Thinking gguf inherits a window sized for reasoning tokens it never emits. If the field is meant to be usable, render it next to each allowlisted path. `NGpuLayers` has the same shape. - Nothing persists which model was swapped in, so a restart silently returns to `phraser.model_path`. That is defensible as policy, and it is invisible on the page. One line saying so would close it. It matters more with PR 69 next in the stack. An update restarts the daemon, so it undoes any swap without saying it did.
kami closed this pull request 2026-08-01 14:51:53 +02:00
Owner

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Pull request closed

Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kami/Maven#68