Label eval runs with the real model; stop lying about killing Maven (#379, #380) #34

Closed
claude wants to merge 4 commits from overnight/eval-label-kill into overnight/delivery-boundary
Contributor

Three commits, ~190 lines. Vikunja #379 and #380 — unrelated code, same bug: both printed something confidently wrong.

#379 — the eval report labelled every run "(0.8B)". Two runs from two different models came out named identically, which is exactly how you mix them up while comparing. It nearly bit during the LFM2.5 bake-off (those numbers are fine — model identity was checked per run by hand).

Turned out the router eval was already fixed on a branch below this one. The same hardcoded label was still in the phrasing eval, so that is what this fixes. The ModelID helper moved to internal/llm — the llama-server client, its natural home — so both eval paths share one copy instead of drifting apart.

When it can't ask the server, it says unknown-model and logs it loudly. That's the whole point: a wrong label is the bug, a blank one isn't. It also now errors on a blank id field rather than quietly labelling a run with an empty string.

One thing worth knowing: it uses its own HTTP client with no proxy, because this box's http_proxy points at a SOCKS bridge that 503s loopback — which would have looked like "the server won't say which model it loaded".

#380kill-maven.sh printed "All services gracefully stopped" while everything kept running. docker-compose.yml doesn't set pid: host, so each container has its own PID namespace and pkill on the host reaches nothing inside them. A false success is worse than not working: you walk away believing it's down.

Now it stops containers through docker compose stop, re-checks, and exits 1 with docker compose ps if any survive. The bare-metal path still works and now also re-verifies after SIGKILL instead of assuming.

The third commit fixes my own overcorrection in the second. "Nothing found on the host" was made to always fail — but that lumps two different things together. If docker answers and names no running containers, Maven really is down; failing there just swaps a false success for a false failure. Now: docker answered ⇒ "nothing to stop", exit 0. Docker couldn't be asked ⇒ unknown, exit 1 with the manual command to run.

A latent footgun found on the way: pgrep -f matches whole command lines, so a shell that merely mentions mavend matched — including this script's own parent. The SIGKILL sweep could have killed the terminal you ran it from. It now drops $$ and $PPID.

How this was verified, since you can't just run it here — the daemons are live on this box, so the script was never executed for real. bash -n is clean, and it was copied to a scratch dir next to a dummy compose file and run against stub docker/pgrep/pkill covering: containers running, containers refusing to stop, docker unreachable with no host processes, docker answering with nothing anywhere, bare-metal SIGTERM working, and bare-metal surviving SIGKILL. Six branches, expected exit code each time.

Two things found and not fixed: Maven isn't actually running as containers on this box right now (only the ecosystem services are), so #380 is about the deploy shape rather than today's state. And the loose llama-server on 18099 — the eval one — matches the script's pattern and would get SIGTERM'd on a bare-metal run. Pre-existing, and the script documents its MODEL escape hatch, so left alone.

Three commits, ~190 lines. Vikunja #379 and #380 — unrelated code, same bug: both printed something confidently wrong. **#379 — the eval report labelled every run "(0.8B)".** Two runs from two different models came out named identically, which is exactly how you mix them up while comparing. It nearly bit during the LFM2.5 bake-off (those numbers are fine — model identity was checked per run by hand). Turned out the **router** eval was already fixed on a branch below this one. The same hardcoded label was still in the **phrasing** eval, so that is what this fixes. The `ModelID` helper moved to `internal/llm` — the llama-server client, its natural home — so both eval paths share one copy instead of drifting apart. **When it can't ask the server, it says `unknown-model` and logs it loudly.** That's the whole point: a wrong label is the bug, a blank one isn't. It also now errors on a blank `id` field rather than quietly labelling a run with an empty string. One thing worth knowing: it uses its own HTTP client with no proxy, because this box's `http_proxy` points at a SOCKS bridge that 503s loopback — which would have looked like "the server won't say which model it loaded". **#380 — `kill-maven.sh` printed "All services gracefully stopped" while everything kept running.** `docker-compose.yml` doesn't set `pid: host`, so each container has its own PID namespace and `pkill` on the host reaches nothing inside them. A false success is worse than not working: you walk away believing it's down. Now it stops containers through `docker compose stop`, **re-checks**, and exits 1 with `docker compose ps` if any survive. The bare-metal path still works and now also re-verifies after SIGKILL instead of assuming. **The third commit fixes my own overcorrection in the second.** "Nothing found on the host" was made to always fail — but that lumps two different things together. If docker answers and names no running containers, Maven really *is* down; failing there just swaps a false success for a false failure. Now: docker answered ⇒ "nothing to stop", exit 0. Docker couldn't be asked ⇒ unknown, exit 1 with the manual command to run. **A latent footgun found on the way:** `pgrep -f` matches whole command lines, so a shell that merely *mentions* mavend matched — including this script's own parent. The SIGKILL sweep could have killed the terminal you ran it from. It now drops `$$` and `$PPID`. **How this was verified, since you can't just run it here** — the daemons are live on this box, so the script was never executed for real. `bash -n` is clean, and it was copied to a scratch dir next to a dummy compose file and run against stub `docker`/`pgrep`/`pkill` covering: containers running, containers refusing to stop, docker unreachable with no host processes, docker answering with nothing anywhere, bare-metal SIGTERM working, and bare-metal surviving SIGKILL. Six branches, expected exit code each time. **Two things found and not fixed:** Maven isn't actually running as containers on this box right now (only the ecosystem services are), so #380 is about the deploy shape rather than today's state. And the loose `llama-server` on 18099 — the eval one — matches the script's pattern and would get SIGTERM'd on a bare-metal run. Pre-existing, and the script documents its `MODEL` escape hatch, so left alone.
claude added 4 commits 2026-07-31 12:33:25 +02:00
The phrasing eval printed "llm (0.8B, ...)" no matter which gguf
llama-server had loaded, so two runs of two different models came out
named the same and were easy to mix up when comparing.

It now asks llama-server over /v1/models, same as the router eval
already did. The helper moved to internal/llm so both share it, and it
now errors instead of returning a blank name when the id field is
missing — an unreachable server gets labelled "unknown-model", never a
plausible-looking guess.

Both eval paths stay opt-in behind MAVEN_LLM_URL; no server needed for
go test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
docker-compose.yml has no 'pid: host', so each container has its own PID
namespace and pkill on the host matches nothing inside them. The script
then printed "All services gracefully stopped" while mavend, its
llama-server and the rest were still running.

Now it checks for running compose containers first and stops them with
docker compose. If it cannot ask docker and finds nothing to kill on the
host, or anything survives the kill, it says so and exits non-zero
instead of claiming success. The bare-metal path is unchanged apart from
verifying the SIGKILL actually worked, and no longer risks killing the
shell it was launched from.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
"Nothing on the host" meant two different things and the script treated
them the same. If docker answers and names no running containers, Maven
really is down and the script should say so and exit 0. Only when docker
cannot be asked is the answer unknown, and that is the case that must
fail loudly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
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:22:28 +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#34