Add a voice.llm_router flag (default off) and fix kill-maven.sh #3

Closed
claude wants to merge 0 commits from overnight/llm-router-flag into master
Contributor

Vikunja #320. Default is off, so nothing changes on the deploy box yet. This is the switch, not the flip.

Commit 1 — the flag. voice.llm_router on VoiceConfig, wired into buildRouter. deploy/mavend.json sets it to false explicitly so you can see it exists.

If the flag is on but there is no llama-server to talk to, Maven logs one line and keeps routing with the classifier. A turn never breaks over this.

The old comment at voice.go:211 said "the classifier handles routing reliably". Last night's eval measured it at 36.8% intent accuracy on held-out utterances, so that comment was false and is now replaced with the real numbers.

Why the default stays off. Two things have to land first, and both are named in a TODO next to the default:

  1. The LLM router hardcodes Confidence: 1.0, so the clarify gate can never fire on an LLM route. Flipping today would not improve the refusal lane, it would delete it (Vikunja #359).
  2. Extractor.Extract never runs on an LLM decision, so acts arrive with no Fn and reminders with no Time. That is the whole 50.0% → 32.9% gap in the eval.

Both are being worked on separately. When they are done, flipping this default is a one-line change.

Commit 2 — kill-maven.sh actually kills the server now. MODEL defaulted to LFM2, but the deploy runs Qwen3.5-0.8B, so the pkill pattern matched nothing and the script silently left llama-server running. Default is now any .gguf this repo serves; set MODEL if you ever need to spare another llama-server.

Vikunja #320. **Default is off, so nothing changes on the deploy box yet.** This is the switch, not the flip. **Commit 1 — the flag.** `voice.llm_router` on `VoiceConfig`, wired into `buildRouter`. `deploy/mavend.json` sets it to `false` explicitly so you can see it exists. If the flag is on but there is no llama-server to talk to, Maven logs one line and keeps routing with the classifier. A turn never breaks over this. The old comment at `voice.go:211` said "the classifier handles routing reliably". Last night's eval measured it at **36.8%** intent accuracy on held-out utterances, so that comment was false and is now replaced with the real numbers. **Why the default stays off.** Two things have to land first, and both are named in a TODO next to the default: 1. The LLM router hardcodes `Confidence: 1.0`, so the clarify gate can never fire on an LLM route. Flipping today would not improve the refusal lane, it would delete it (Vikunja #359). 2. `Extractor.Extract` never runs on an LLM decision, so acts arrive with no `Fn` and reminders with no `Time`. That is the whole 50.0% → 32.9% gap in the eval. Both are being worked on separately. When they are done, flipping this default is a one-line change. **Commit 2 — `kill-maven.sh` actually kills the server now.** `MODEL` defaulted to `LFM2`, but the deploy runs `Qwen3.5-0.8B`, so the pkill pattern matched nothing and the script silently left llama-server running. Default is now any `.gguf` this repo serves; set `MODEL` if you ever need to spare another llama-server.
claude added 2 commits 2026-07-31 00:17:33 +02:00
Wires cmd/mavend/voice.go to build the LLM router when the operator asks
for it. Default false, so nothing changes on the deploy box.
Look at pickLLMRouter: the flag on with no llama-server logs one line and
keeps the classifier, it never fails a turn.
The default stays off until the router can refuse (#359) and the extractor
runs on LLM decisions — both noted as TODOs in config.go.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
The MODEL default was LFM2, but the deploy runs Qwen3.5-0.8B, so the
pkill pattern matched nothing and the server survived every kill.
Now matches any llama-server serving a .gguf, so changing the model in
deploy/mavend.json cannot break the script again. MODEL still narrows it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
kami reviewed 2026-07-31 10:01:43 +02:00
@@ -14,2 +19,3 @@
MODEL="${MODEL:-}"
PAT='mavend|mavsttd|mavttsd|mavweb|mavpoll|mavenclient'
LLM="llama-server.*${MODEL}"
if [ -n "$MODEL" ]; then
Owner

so, do I get it right - we either pick an already running llama-server or spawn a new one with the specified model?
in Docker container, btw.

so, do I get it right - we either pick an already running llama-server or spawn a new one with the specified model? in Docker container, btw.
Owner

overall looks good, but not sure about the llama-server script in there.

overall looks good, but not sure about the llama-server script in there.
Author
Contributor

Not quite — and the confusion is my fault for not saying what the script does at the top.

kill-maven.sh only kills. It never picks up a running server and never spawns one. There's no "reuse or start" logic anywhere in it.

What actually happens: mavend always spawns its own llama-server as a child process, from phraser.model_path in deploy/mavend.json. It never attaches to one that's already running. So there's exactly one server per mavend, and it dies with mavend (Pdeathsig).

The script exists for the case where that didn't happen — mavend was killed hard, and its llama-server is orphaned with nobody left to reap it. Those orphans hold a couple of GB each and were OOMing the box. All the pattern-matching you're looking at is just "find any llama-server serving a .gguf", because the old pattern matched on the model name and stopped matching the moment the deploy switched from LFM2 to Qwen.

Your Docker point is a real bug and I'm glad you asked. The compose file doesn't set pid: host, so each container has its own PID namespace. Run this script on the host and pkill sees nothing inside the containers — it'll cheerfully print "All services gracefully stopped" while everything is still running. It only works for a bare-metal or dev run.

That's a wrong answer, not just a missing feature, so I'll fix it separately: detect the containerised case and use docker compose stop, or at minimum refuse to claim success when it can't see the processes. Filing it now.

Not quite — and the confusion is my fault for not saying what the script does at the top. `kill-maven.sh` **only kills**. It never picks up a running server and never spawns one. There's no "reuse or start" logic anywhere in it. What actually happens: **mavend always spawns its own llama-server** as a child process, from `phraser.model_path` in `deploy/mavend.json`. It never attaches to one that's already running. So there's exactly one server per mavend, and it dies with mavend (Pdeathsig). The script exists for the case where that didn't happen — mavend was killed hard, and its llama-server is orphaned with nobody left to reap it. Those orphans hold a couple of GB each and were OOMing the box. All the pattern-matching you're looking at is just "find any llama-server serving a .gguf", because the old pattern matched on the model name and stopped matching the moment the deploy switched from LFM2 to Qwen. **Your Docker point is a real bug and I'm glad you asked.** The compose file doesn't set `pid: host`, so each container has its own PID namespace. Run this script on the host and `pkill` sees nothing inside the containers — it'll cheerfully print "All services gracefully stopped" while everything is still running. It only works for a bare-metal or dev run. That's a wrong answer, not just a missing feature, so I'll fix it separately: detect the containerised case and use `docker compose stop`, or at minimum refuse to claim success when it can't see the processes. Filing it now.
kami reviewed 2026-07-31 20:18:37 +02:00
kami left a comment
Owner

It spawns its own, always — there is no reuse path in the daemon, and in Docker it is a child inside the mavend container.

mavend boots the phraser through phraser.NewLLMPhraser, which unconditionally calls start() (internal/phraser/llmphraser.go:85-97) and exec.CommandContexts llama-server (:129-147). It never probes the port first, so an already-running server on the box is neither found nor used. The one reuse path, NewLLMPhraserAt (:100-115), exists only for internal/phraser/eval — it attaches to a server someone else owns and its Close kills nothing. The daemon does not use it.

Two consequences that shaped this script:

  1. It cannot be found by port. The default Listen is 127.0.0.1:0 (DefaultConfig, :78) — an ephemeral port read back out of llama-server's stderr (listenRE, :168). Nothing else can discover it, which is also why kill-maven.sh matches on the command line rather than on a port.
  2. In Docker it is inside the container. docker-compose.yml runs mavend from the shared image with command: ["mavend", ...]; the llama-server it spawns is that container's child, in the container's own PID namespace (no pid: host). pkill on the host cannot see it at all — that is exactly why this script goes through docker compose stop when containers are up, and only falls through to the pgrep/pkill sweep for a bare-metal dev run.

The host sweep still matters even with Pdeathsig: SIGKILL (:146): the kernel kills the child when mavend dies, but an orphan from a mavend that already died before the Pdeathsig work landed, or one started by hand for an eval run, has nobody to reap it. Hence llama-server.*\\.gguf rather than a model name.

It spawns its own, always — there is no reuse path in the daemon, and in Docker it is a child inside the `mavend` container. `mavend` boots the phraser through `phraser.NewLLMPhraser`, which unconditionally calls `start()` (`internal/phraser/llmphraser.go:85-97`) and `exec.CommandContext`s `llama-server` (:129-147). It never probes the port first, so an already-running server on the box is neither found nor used. The one reuse path, `NewLLMPhraserAt` (:100-115), exists only for `internal/phraser/eval` — it attaches to a server someone else owns and its `Close` kills nothing. The daemon does not use it. Two consequences that shaped this script: 1. **It cannot be found by port.** The default `Listen` is `127.0.0.1:0` (`DefaultConfig`, :78) — an ephemeral port read back out of llama-server's stderr (`listenRE`, :168). Nothing else can discover it, which is also why `kill-maven.sh` matches on the command line rather than on a port. 2. **In Docker it is inside the container.** `docker-compose.yml` runs `mavend` from the shared image with `command: ["mavend", ...]`; the llama-server it spawns is that container's child, in the container's own PID namespace (no `pid: host`). `pkill` on the host cannot see it at all — that is exactly why this script goes through `docker compose stop` when containers are up, and only falls through to the `pgrep`/`pkill` sweep for a bare-metal dev run. The host sweep still matters even with `Pdeathsig: SIGKILL` (:146): the kernel kills the child when *mavend* dies, but an orphan from a `mavend` that already died before the Pdeathsig work landed, or one started by hand for an eval run, has nobody to reap it. Hence `llama-server.*\\.gguf` rather than a model name.
Owner

Superseded by #47, which landed this whole stack on master as one reviewed integration merge. This PR head is an ancestor of master — its commits are in, nothing here is lost. Closing as merged-by-proxy rather than merged, since the merge came in through #47.

Review threads on this PR were answered or acted on before the merge; the Russian wording fixes went in as #48.

Superseded by #47, which landed this whole stack on master as one reviewed integration merge. This PR head is an ancestor of master — its commits are in, nothing here is lost. Closing as merged-by-proxy rather than merged, since the merge came in through #47. Review threads on this PR were answered or acted on before the merge; the Russian wording fixes went in as #48.
kami closed this pull request 2026-07-31 20:21:32 +02:00

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#3