Merge the eval label and kill script fixes

This commit is contained in:
kami
2026-07-31 14:32:45 +04:00
5 changed files with 184 additions and 12 deletions
@@ -1,4 +1,4 @@
package eval
package llm
import (
"context"
@@ -6,8 +6,20 @@ import (
"fmt"
"net/http"
"strings"
"time"
)
// UnknownModel is the label to print when the server would not say what it has
// loaded. Deliberately ugly: an honest "unknown" is fine, a plausible-looking
// but wrong model name is the bug this whole file exists to prevent.
const UnknownModel = "unknown-model"
// llama-server is local, so never send this through a proxy: this box's
// http_proxy answers 503 for loopback, which would look like "server won't say
// which model it has" when the server is right there and fine.
// A Transport with no Proxy set bypasses http_proxy entirely.
var modelHTTP = &http.Client{Timeout: 10 * time.Second, Transport: &http.Transport{}}
// ModelID asks llama-server which model it has loaded, so a scoring run can
// label itself. Without this a bake-off between two models produces two tables
// that look identical, and the operator has to remember which server was up.
@@ -19,7 +31,7 @@ func ModelID(ctx context.Context, base string) (string, error) {
if err != nil {
return "", err
}
resp, err := http.DefaultClient.Do(req)
resp, err := modelHTTP.Do(req)
if err != nil {
return "", err
}
@@ -38,14 +50,21 @@ func ModelID(ctx context.Context, base string) (string, error) {
if len(out.Data) == 0 {
return "", fmt.Errorf("models: empty list")
}
return shortModelID(out.Data[0].ID), nil
short := shortModelID(out.Data[0].ID)
if short == "" {
// Server answered but the id field was missing or blank. Say so
// instead of handing back an empty label that reads as a real name.
return "", fmt.Errorf("models: no id in response")
}
return short, nil
}
// shortModelID trims the path and the .gguf suffix — llama-server reports the
// file name it was started with, which is too long for a table header.
func shortModelID(id string) string {
id = strings.TrimSpace(id)
if i := strings.LastIndexAny(id, "/\\"); i >= 0 {
id = id[i+1:]
}
return strings.TrimSuffix(id, ".gguf")
return strings.TrimSpace(strings.TrimSuffix(id, ".gguf"))
}
+64
View File
@@ -0,0 +1,64 @@
package llm
import (
"context"
"net/http"
"net/http/httptest"
"testing"
)
// The point of these tests: a wrong-but-plausible model label is the bug, so
// every path that cannot learn the real name must return an error instead of a
// guess. No llama-server needed — a stub server stands in.
func TestModelID(t *testing.T) {
cases := []struct {
name string
body string
code int
want string // "" ⇒ expect an error
}{
{"full path", `{"data":[{"id":"/mnt/hdd1/llms/qwen3.5/Qwen3.5-0.8B.Q4_K_M.gguf"}]}`, 200, "Qwen3.5-0.8B.Q4_K_M"},
{"bare name", `{"data":[{"id":"LFM2.5-1.2B"}]}`, 200, "LFM2.5-1.2B"},
{"empty list", `{"data":[]}`, 200, ""},
{"id missing", `{"data":[{}]}`, 200, ""},
{"id blank", `{"data":[{"id":" "}]}`, 200, ""},
{"server error", `nope`, 500, ""},
{"not json", `<html>`, 200, ""},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/v1/models" {
t.Errorf("asked for %s, want /v1/models", r.URL.Path)
}
w.WriteHeader(c.code)
_, _ = w.Write([]byte(c.body))
}))
defer srv.Close()
got, err := ModelID(context.Background(), srv.URL+"/")
if c.want == "" {
if err == nil {
t.Fatalf("want an error, got label %q", got)
}
return
}
if err != nil {
t.Fatalf("ModelID: %v", err)
}
if got != c.want {
t.Errorf("got %q, want %q", got, c.want)
}
})
}
}
func TestModelIDUnreachable(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}))
url := srv.URL
srv.Close() // nothing listening now
if got, err := ModelID(context.Background(), url); err == nil {
t.Fatalf("want an error from a dead server, got label %q", got)
}
}
+15 -1
View File
@@ -7,6 +7,7 @@ import (
"testing"
"time"
"github.com/kami/maven/internal/llm"
"github.com/kami/maven/internal/phraser"
)
@@ -32,6 +33,7 @@ func TestLLMPhrasingBaseline(t *testing.T) {
// case as a phrasing error and read as "the model cannot phrase".
noProxyLoopback(t)
ctx := context.Background()
f, err := Load()
if err != nil {
t.Fatalf("Load: %v", err)
@@ -44,7 +46,19 @@ func TestLLMPhrasingBaseline(t *testing.T) {
p := phraser.NewLLMPhraserAt(base, cfg)
defer p.Close()
rep, err := Score(context.Background(), "llm (0.8B, built-in persona)", p, f)
// Label the run with whatever gguf the server actually has loaded. It used
// to say "0.8B" no matter what, so two runs of two different models came
// out named the same and were easy to mix up when comparing.
model, err := llm.ModelID(ctx, base)
if err != nil {
// An unlabelled score is still a score, but say so loudly — a made-up
// name in a bake-off table is worse than no name.
t.Logf("could not read model id from %s: %v — report will say %q", base, err, llm.UnknownModel)
model = llm.UnknownModel
}
t.Logf("scoring model %s at %s", model, base)
rep, err := Score(ctx, "llm ("+model+", built-in persona)", p, f)
if err != nil {
t.Fatalf("Score: %v", err)
}
+3 -3
View File
@@ -61,12 +61,12 @@ func TestLLMRouterBaseline(t *testing.T) {
}
ctx := context.Background()
model, err := ModelID(ctx, base)
model, err := llm.ModelID(ctx, base)
if err != nil {
// Not fatal: an unlabelled score is still a score. But say so loudly,
// because an unlabelled row in a bake-off table is worthless.
t.Logf("could not read model id from %s: %v — reports will say %q", base, err, "unknown-model")
model = "unknown-model"
t.Logf("could not read model id from %s: %v — reports will say %q", base, err, llm.UnknownModel)
model = llm.UnknownModel
}
t.Logf("scoring model %s at %s", model, base)
lr := router.NewLLMRouter(client)
+79 -4
View File
@@ -1,8 +1,10 @@
#!/usr/bin/env bash
# Unified script to stop all Maven services.
# Usage: ./kill-maven.sh
# - Graceful SIGTERM is attempted first.
# - If any process lingers, force with SIGKILL.
# - Docker deploy: `docker compose stop` (see why below).
# - Bare-metal / dev run: graceful SIGTERM first, SIGKILL if anything lingers.
# Exits non-zero if it cannot confirm everything is stopped. It must never say
# "stopped" unless it checked.
set -euo pipefail
@@ -24,6 +26,73 @@ else
LLM='llama-server.*\.gguf'
fi
COMPOSE_FILE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/docker-compose.yml"
# --- containerised deploy ------------------------------------------------
# docker-compose.yml does not set `pid: host`, so each container has its own
# PID namespace: pkill on the host sees nothing inside them. This script used
# to print "all stopped" while every daemon was still happily running. Stop the
# containers through compose instead — that actually reaches them.
#
# running_containers prints the ids of the project's running containers, or
# nothing. Empty output plus a non-zero return means "could not ask docker",
# which is different from "nothing is running" and is handled below.
running_containers() {
docker compose -f "$COMPOSE_FILE" ps -q --status running 2>/dev/null
}
DOCKER_OK=0
CONTAINERS=""
if command -v docker >/dev/null 2>&1 && [ -f "$COMPOSE_FILE" ]; then
if CONTAINERS="$(running_containers)"; then
DOCKER_OK=1
fi
fi
if [ "$DOCKER_OK" = 1 ] && [ -n "$CONTAINERS" ]; then
echo "--- Maven is running in containers: stopping via docker compose ---"
if ! docker compose -f "$COMPOSE_FILE" stop; then
echo "ERROR: 'docker compose stop' failed. Containers may still be running." >&2
exit 1
fi
echo "--- Verifying containers are gone ---"
LEFT="$(running_containers || true)"
if [ -n "$LEFT" ]; then
echo "ERROR: containers still running after stop:" >&2
docker compose -f "$COMPOSE_FILE" ps >&2 || true
exit 1
fi
echo "All containers stopped."
exit 0
fi
# --- bare-metal / dev run -----------------------------------------------
# pgrep -f matches whole command lines, so a shell that merely mentions
# "mavend" (this script's own parent, for one) shows up. Drop ourselves and our
# parent, otherwise the SIGKILL sweep can take out the terminal you ran this in.
host_pids() {
pgrep -f "$PAT|$LLM" | grep -v -e "^$$\$" -e "^$PPID\$" | paste -sd, - || true
}
HOST_PIDS=$(host_pids)
if [ -z "$HOST_PIDS" ]; then
# Nothing on the host. Whether that means "already down" depends on whether
# we managed to ask docker, and the two must not read the same.
if [ "$DOCKER_OK" = 1 ]; then
# Docker answered and named no running containers, and there is nothing
# on the host either. That is a real answer: Maven is already stopped.
echo "Nothing to stop: no Maven processes and no running containers."
exit 0
fi
# We could not ask docker, so Maven may be alive in a container we cannot
# see. Saying "stopped" here is the exact false success this script had.
echo "ERROR: no Maven processes on this host, and docker could not be asked." >&2
echo " If this is the container deploy it may still be running:" >&2
echo " docker compose -f $COMPOSE_FILE stop" >&2
echo " Nothing was stopped. Check by hand before assuming Maven is down." >&2
exit 1
fi
echo "--- Sending graceful SIGTERM to Maven services ---"
pkill -TERM -f "$PAT" || true
# mavend's Pdeathsig SIGKILLs its llama-server on exit, but sweep strays too
@@ -32,12 +101,18 @@ pkill -TERM -f "$LLM" || true
echo "--- Verifying processes are gone ---"
sleep 1
PIDS=$(pgrep -d ',' -f "$PAT|$LLM") || PIDS=""
PIDS=$(host_pids)
if [ -n "$PIDS" ]; then
echo "Warning: some processes still alive. PIDs: $PIDS"
echo "--- Force killing with SIGKILL ---"
echo "$PIDS" | tr ',' '\n' | xargs -r kill -9
sleep 1
LEFT=$(host_pids)
if [ -n "$LEFT" ]; then
echo "ERROR: still alive after SIGKILL. PIDs: $LEFT" >&2
exit 1
fi
echo "Done (SIGKILL)."
else
echo "All services gracefully stopped."
fi
fi