From d596ec8abf0a92b5b761c994afeb9cf185889d1d Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Fri, 25 Sep 2026 11:22:37 -0700 Subject: [PATCH] =?UTF-8?q?v2.1=20task=2002:=20router=20mode=20=E2=80=94?= =?UTF-8?q?=20status=20loaded=20only,=20per-model=20/props,=20guard=20and?= =?UTF-8?q?=20hosts=20view=20use=20it;=20given=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5.1 --- docs/plans/v2.1/02-props-loaded-only.md | 93 +++++++++ docs/plans/v2.1/README.md | 8 +- .../internal/admin/admin_models_test.go | 46 +++++ .../internal/health/props_router_test.go | 177 ++++++++++++++++++ .../internal/proxy/ctxguard_router_test.go | 105 +++++++++++ 5 files changed, 426 insertions(+), 3 deletions(-) create mode 100644 docs/plans/v2.1/02-props-loaded-only.md create mode 100644 docs/plans/v2.1/_files/internal/admin/admin_models_test.go create mode 100644 docs/plans/v2.1/_files/internal/health/props_router_test.go create mode 100644 docs/plans/v2.1/_files/internal/proxy/ctxguard_router_test.go diff --git a/docs/plans/v2.1/02-props-loaded-only.md b/docs/plans/v2.1/02-props-loaded-only.md new file mode 100644 index 0000000..700220e --- /dev/null +++ b/docs/plans/v2.1/02-props-loaded-only.md @@ -0,0 +1,93 @@ +# v2.1 task 02: router mode — loaded means loaded, context is per model + +**Branch:** `v2.1` (`git switch v2.1`; `git status --short` must be empty, otherwise stop) +**Commit subject:** `Learn per-model context from /props?model=; only status "loaded" is loaded` + +## Goal + +crossbar's real upstreams are llama-server **routers**, not single servers, and v2's poller was +written against the single-server shape. On a router: `/v1/models` lists every configured model +with a `status.value` (`"loaded"`, `"unloaded"`, `"loading"`); the plain `/props` answers as the +router itself (`"role":"router"`, `n_ctx` 0); and `/props?model=X` answers for X's child server — +**and loads X if it is not loaded**, which a health poll must never cause. After this task the +poller treats only `"loaded"` models as loaded, asks `/props?model=X` only for those, keeps the +answers per model, and the guard and the hosts view use the per-model figures. A plain single +server keeps working exactly as in v2. + +## Context + +Verified on straylight's router on 2026-09-25: `/v1/models` entries carry +`"status":{"value":"unloaded",...}` for seven of eight models; plain `/props` returns +`{"role":"router","model_alias":"llama-server","default_generation_settings":{"n_ctx":0}}`; +`/props?model=ornith-1.5-35b-a3b` (loaded) returns `n_ctx` 262144 and `total_slots` 4. With v2's +code every listed model counts as loaded and the guard learns nothing, so the "Context size has +been exceeded" failure crossbar exists to prevent still happens on a router. + +## Files + +- Copy: `internal/health/props_router_test.go`, `internal/proxy/ctxguard_router_test.go`, + `internal/admin/admin_models_test.go` +- Modify: `internal/health/health.go` (a new `internal/health/props.go` is allowed for the 400-line + limit), `internal/proxy/ctxguard.go`, `internal/admin/admin.go`, `docs/implementer-log.md` + +## Interfaces + +```go +package health + +// ModelCtx is what /props?model=X taught us about one loaded model. +type ModelCtx struct { + NCtx int `json:"n_ctx"` + Slots int `json:"slots"` +} + +type Status struct { + // ... as v2 ... + Models map[string]ModelCtx `json:"models"` // per loaded model; never nil after a poll; copied by Get/All +} + +// PerSlotCtxFor is the per-slot context for one model on this host: Models[model] when present +// (NCtx/Slots, 0 when either is 0); else, when model is in Loaded, the host-level PerSlotCtx(); +// else 0 ("unknown" / not resident). +func (s Status) PerSlotCtxFor(model string) int +``` + +Poller rules: +1. `/v1/models`: an entry is loaded when it has no `status` or `status.value == "loaded"`; any + other value (`"unloaded"`, `"loading"`, …) is **not loaded** and does not appear in `Loaded`. +2. Plain `/props`: when the body has `"role":"router"` the host-level `NCtx`/`Slots` stay 0 + whatever else it says; otherwise as v2 (best effort, never a failure). +3. For each model in `Loaded`, `GET /props?model=`, decoded like the plain + one, into `Models[id]`. A failed or malformed answer leaves that id absent and the host + healthy. Never ask for a model that is not in `Loaded`. +4. `Models` is a fresh non-nil map on every successful poll; `MarkDown` leaves it as last seen. + +Proxy (`ctxguard.go`): every use of `PerSlotCtx()` becomes `PerSlotCtxFor(model)` — the leased +host's figure, the candidates a prompt may move to, the wake path's "cannot serve it no matter +how it wakes" check, and `largestSlotCtx`, which now takes the model and so only counts hosts +that have it loaded. Admin (`admin.go`): `HostView` gains `Models map[string]health.ModelCtx` +(JSON `models`, an empty object never `null`). + +## Steps + +- [ ] **1.** `git switch v2.1`; copy the three given tests. +- [ ] **2. See them fail:** `go test -race -count=1 ./internal/health/ ./internal/proxy/ ./internal/admin/` — the new tests fail to compile until the names exist, then fail on behaviour. +- [ ] **3.** `health` first (rules 1–4, `ModelCtx`, `PerSlotCtxFor`); `go test -race -count=1 ./internal/health/` → `ok`. +- [ ] **4.** `ctxguard.go`, then `admin.go`; each package `ok`. `gofmt -w`. +- [ ] **5.** `go test -race -count=1 ./...` → all `ok`; `make smoke` → `smoke: ok` (the smoke's fake is a single server; nothing there changes). +- [ ] **6.** `make gate`. **7.** Row `v2.1/02-props-loaded-only`; commit. + +```sh +git add internal/health internal/proxy internal/admin docs/implementer-log.md +git commit +``` + +## Done when + +- All three given tests pass; every v2 health/guard/admin test still passes; gate and smoke ok; + given files byte-identical; no file over 400 lines. + +## Stop and report if + +- A v2 given test (`props_test.go`, `ctxguard_test.go`, `admin_test.go`, `health_test.go`) needs + changing to pass: quote it — that is the owner's test, not yours to edit. diff --git a/docs/plans/v2.1/README.md b/docs/plans/v2.1/README.md index 3b256f7..28e8647 100644 --- a/docs/plans/v2.1/README.md +++ b/docs/plans/v2.1/README.md @@ -9,9 +9,11 @@ the reverse proxy observed. Found 2026-09-25 by the intermittent `Errors:2` in `TestQueueFullIs503`; verified with a diagnostic build; the given `internal/proxy/served_test.go` reproduces it on every run. -- **02-props-loaded-only** (to be written after v2 merges) — the poller asks `/props?model=X` - only for models `/v1/models` lists as loaded, because llama-server's router autoloads a model - named in that query (`models_autoload`). See the v2 README's note of 2026-09-25. +- **02-props-loaded-only** — on a router only `status.value == "loaded"` counts as loaded; the + poller asks `/props?model=X` only for those (the router autoloads a model named in that query) + and keeps the answers per model (`Status.Models`, `PerSlotCtxFor`); the guard and the hosts view + use them. Facts verified against straylight's router on 2026-09-25. Given tests: + `health/props_router_test.go`, `proxy/ctxguard_router_test.go`, `admin/admin_models_test.go`. - ~~03-timing-margins~~ — done by the owner directly (test-only work, no implementer task): the remaining ordering sleeps in `limiter_test.go`, `proxy_test.go` and `cancel_test.go` now wait on limiter state (`Queued`/`InFlight`); the one sleep left is a deliberate lower bound. Five clean diff --git a/docs/plans/v2.1/_files/internal/admin/admin_models_test.go b/docs/plans/v2.1/_files/internal/admin/admin_models_test.go new file mode 100644 index 0000000..b9d472c --- /dev/null +++ b/docs/plans/v2.1/_files/internal/admin/admin_models_test.go @@ -0,0 +1,46 @@ +package admin_test + +import ( + "encoding/json" + "strings" + "testing" + + "git.wntrmute.dev/kyle/crossbar/internal/admin" + "git.wntrmute.dev/kyle/crossbar/internal/health" +) + +// The hosts view shows the per-model context the poller learned, and an empty object (never +// null) for a host with nothing learned. +func TestHostsShowsPerModelContext(t *testing.T) { + r := newRig(t) + r.hosts.st["alpha"] = health.Status{ + Healthy: true, + Loaded: []string{"m"}, + NCtx: 0, // a router: the host-level figure stays unknown + Models: map[string]health.ModelCtx{"m": {NCtx: 65536, Slots: 2}}, + } + rec := r.do(t, "GET", "/_crossbar/hosts", "") + if rec.Code != 200 { + t.Fatalf("%d %s", rec.Code, rec.Body.String()) + } + var out map[string]admin.HostView + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatal(err) + } + if got := out["alpha"].Models["m"]; got != (health.ModelCtx{NCtx: 65536, Slots: 2}) { + t.Errorf("alpha.models[m] = %+v, want {65536 2}", got) + } + if out["alpha"].NCtx != 0 { + t.Errorf("alpha.n_ctx = %d, want 0 (unknown at host level on a router)", out["alpha"].NCtx) + } + var raw map[string]json.RawMessage + if err := json.Unmarshal(rec.Body.Bytes(), &raw); err != nil { + t.Fatal(err) + } + if beta := string(raw["beta"]); !strings.Contains(beta, `"models":{}`) { + t.Errorf("beta = %s, want \"models\":{} (never null)", beta) + } + if alpha := string(raw["alpha"]); !strings.Contains(alpha, `"models":{"m":{"n_ctx":65536,"slots":2}}`) { + t.Errorf("alpha = %s, want models keyed by id with n_ctx and slots", alpha) + } +} diff --git a/docs/plans/v2.1/_files/internal/health/props_router_test.go b/docs/plans/v2.1/_files/internal/health/props_router_test.go new file mode 100644 index 0000000..f74bd52 --- /dev/null +++ b/docs/plans/v2.1/_files/internal/health/props_router_test.go @@ -0,0 +1,177 @@ +package health_test + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "sync" + "testing" + "time" + + "git.wntrmute.dev/kyle/crossbar/internal/health" +) + +// routerFake is shaped like llama-server's router mode: /v1/models lists every configured model +// with a status, a plain /props answers as the router itself (no context), and /props?model=X +// answers for one loaded child server. It counts the per-model /props queries it receives. +type routerFake struct { + srv *httptest.Server + mu sync.Mutex + queries map[string]int +} + +func newRouterFake(t *testing.T) *routerFake { + f := &routerFake{queries: map[string]int{}} + mux := http.NewServeMux() + mux.HandleFunc("/health", func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, `{"status":"ok"}`) }) + mux.HandleFunc("/v1/models", func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `{"object":"list","data":[ + {"id":"big","object":"model","status":{"value":"loaded","args":["--ctx-size","262144"]}}, + {"id":"small","object":"model","status":{"value":"loaded"}}, + {"id":"cold","object":"model","status":{"value":"unloaded"}}, + {"id":"warming","object":"model","status":{"value":"loading"}}]}`) + }) + mux.HandleFunc("/props", func(w http.ResponseWriter, r *http.Request) { + model := r.URL.Query().Get("model") + if model == "" { + fmt.Fprint(w, `{"role":"router","model_alias":"llama-server","model_path":"none","default_generation_settings":{"params":null,"n_ctx":0}}`) + return + } + f.mu.Lock() + f.queries[model]++ + f.mu.Unlock() + switch model { + case "big": + fmt.Fprint(w, `{"default_generation_settings":{"n_ctx":262144,"params":{}},"total_slots":4,"model_alias":"big"}`) + case "small": + fmt.Fprint(w, `{"default_generation_settings":{"n_ctx":32768,"params":{}},"total_slots":1,"model_alias":"small"}`) + default: + // Asking a router for an unloaded model would make it load the model. The fake + // answers 500 so a wrong query is visible in the counts and cannot look like success. + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprint(w, `{"error":"the poller must not ask for a model that is not loaded"}`) + } + }) + f.srv = httptest.NewServer(mux) + t.Cleanup(f.srv.Close) + return f +} + +func (f *routerFake) count(model string) int { + f.mu.Lock() + defer f.mu.Unlock() + return f.queries[model] +} + +func TestRouterLoadedMeansStatusLoaded(t *testing.T) { + f := newRouterFake(t) + tbl := health.New(map[string]string{"r": f.srv.URL}, time.Hour, nil) + tbl.PollOnce(context.Background()) + s, ok := tbl.Get("r") + if !ok || !s.Healthy { + t.Fatalf("status = %+v, want a healthy host", s) + } + if len(s.Loaded) != 2 || s.Loaded[0] != "big" || s.Loaded[1] != "small" { + t.Errorf("Loaded = %v, want [big small]: unloaded and loading models are not loaded", s.Loaded) + } +} + +func TestRouterContextIsLearnedPerModel(t *testing.T) { + f := newRouterFake(t) + tbl := health.New(map[string]string{"r": f.srv.URL}, time.Hour, nil) + tbl.PollOnce(context.Background()) + s, _ := tbl.Get("r") + if s.NCtx != 0 || s.Slots != 0 || s.PerSlotCtx() != 0 { + t.Errorf("a router's own /props carries no context; host-level must stay unknown: %+v", s) + } + if got := s.Models["big"]; got != (health.ModelCtx{NCtx: 262144, Slots: 4}) { + t.Errorf("Models[big] = %+v, want {262144 4}", got) + } + if got := s.Models["small"]; got != (health.ModelCtx{NCtx: 32768, Slots: 1}) { + t.Errorf("Models[small] = %+v, want {32768 1}", got) + } + if got := s.PerSlotCtxFor("big"); got != 65536 { + t.Errorf("PerSlotCtxFor(big) = %d, want 262144/4", got) + } + if got := s.PerSlotCtxFor("small"); got != 32768 { + t.Errorf("PerSlotCtxFor(small) = %d, want 32768/1", got) + } + if got := s.PerSlotCtxFor("cold"); got != 0 { + t.Errorf("PerSlotCtxFor(cold) = %d, want 0: nothing is known about an unloaded model", got) + } + if _, present := s.Models["cold"]; present { + t.Errorf("Models must not carry an entry for an unloaded model: %+v", s.Models) + } +} + +func TestRouterUnloadedModelsAreNeverQueried(t *testing.T) { + f := newRouterFake(t) + tbl := health.New(map[string]string{"r": f.srv.URL}, time.Hour, nil) + for i := 0; i < 3; i++ { + tbl.PollOnce(context.Background()) + } + if f.count("cold") != 0 || f.count("warming") != 0 { + t.Fatalf("/props?model= was asked for a model that is not loaded (cold %d, warming %d): on a real router that loads the model", f.count("cold"), f.count("warming")) + } + if f.count("big") == 0 || f.count("small") == 0 { + t.Errorf("loaded models must be asked: big %d, small %d", f.count("big"), f.count("small")) + } +} + +func TestPlainServerStillReadsHostLevelContext(t *testing.T) { + // A single llama-server (no status field, no router role) behaves as in v2: every listed model + // is loaded, the host-level context comes from the plain /props, and the per-model view falls + // back to it for any loaded model. + srv := propsFake(t, `{"default_generation_settings":{"n_ctx":131072,"params":{}},"total_slots":4,"model_path":"/x/m.gguf"}`, 200) + tbl := health.New(map[string]string{"a": srv.URL}, time.Hour, nil) + tbl.PollOnce(context.Background()) + s, _ := tbl.Get("a") + if !s.Healthy || len(s.Loaded) != 1 || s.Loaded[0] != "m" || s.NCtx != 131072 || s.Slots != 4 { + t.Fatalf("status = %+v, want healthy, Loaded [m], NCtx 131072, Slots 4", s) + } + if got := s.PerSlotCtxFor("m"); got != 32768 { + t.Errorf("PerSlotCtxFor(m) = %d, want the host-level 131072/4", got) + } + if got := s.PerSlotCtxFor("other"); got != 0 { + t.Errorf("PerSlotCtxFor(other) = %d, want 0 for a model the host does not list", got) + } + if s.Models == nil { + t.Errorf("Models must be an empty map after a poll, never nil") + } +} + +func TestPerModelPropsFailureLeavesTheModelUnknown(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/health", func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, `{"status":"ok"}`) }) + mux.HandleFunc("/v1/models", func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `{"data":[{"id":"ok","status":{"value":"loaded"}},{"id":"broken","status":{"value":"loaded"}}]}`) + }) + mux.HandleFunc("/props", func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Query().Get("model") { + case "": + fmt.Fprint(w, `{"role":"router","default_generation_settings":{"n_ctx":0}}`) + case "ok": + fmt.Fprint(w, `{"default_generation_settings":{"n_ctx":8192},"total_slots":2}`) + default: + fmt.Fprint(w, `not json`) + } + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + tbl := health.New(map[string]string{"r": srv.URL}, time.Hour, nil) + tbl.PollOnce(context.Background()) + s, _ := tbl.Get("r") + if !s.Healthy { + t.Fatalf("a broken per-model /props must not make the host unhealthy: %+v", s) + } + if len(s.Loaded) != 2 { + t.Errorf("Loaded = %v, want both models: /props is advisory", s.Loaded) + } + if got := s.PerSlotCtxFor("ok"); got != 4096 { + t.Errorf("PerSlotCtxFor(ok) = %d, want 8192/2", got) + } + if _, present := s.Models["broken"]; present || s.PerSlotCtxFor("broken") != 0 { + t.Errorf("a model whose /props failed stays unknown: %+v", s.Models) + } +} diff --git a/docs/plans/v2.1/_files/internal/proxy/ctxguard_router_test.go b/docs/plans/v2.1/_files/internal/proxy/ctxguard_router_test.go new file mode 100644 index 0000000..7c4ffb1 --- /dev/null +++ b/docs/plans/v2.1/_files/internal/proxy/ctxguard_router_test.go @@ -0,0 +1,105 @@ +package proxy_test + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "git.wntrmute.dev/kyle/crossbar/internal/proxy" +) + +// routerUpstream is a fake shaped like llama-server's router mode: the plain /props carries no +// context (role router, n_ctx 0), /v1/models lists models with a status, and /props?model=X +// answers for one loaded model. The guard must work from the per-model figures. +func routerUpstream(t *testing.T, name string, models map[string][2]int, unloaded ...string) *upstream { + u := &upstream{name: name} + mux := http.NewServeMux() + mux.HandleFunc("/health", func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, `{"status":"ok"}`) }) + mux.HandleFunc("/v1/models", func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `{"object":"list","data":[`) + first := true + for id := range models { + if !first { + fmt.Fprint(w, ",") + } + first = false + fmt.Fprintf(w, `{"id":%q,"status":{"value":"loaded"}}`, id) + } + for _, id := range unloaded { + fmt.Fprintf(w, `,{"id":%q,"status":{"value":"unloaded"}}`, id) + } + fmt.Fprint(w, `]}`) + }) + mux.HandleFunc("/props", func(w http.ResponseWriter, r *http.Request) { + model := r.URL.Query().Get("model") + if model == "" { + fmt.Fprint(w, `{"role":"router","default_generation_settings":{"n_ctx":0}}`) + return + } + m, ok := models[model] + if !ok { + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprint(w, `{"error":"asked for a model that is not loaded"}`) + return + } + fmt.Fprintf(w, `{"default_generation_settings":{"n_ctx":%d},"total_slots":%d}`, m[0], m[1]) + }) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + u.hits.Add(1) + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"choices":[{"message":{"role":"assistant","content":"ok"}}],"usage":{"prompt_tokens":1,"completion_tokens":1}}`) + }) + u.srv = httptest.NewServer(mux) + t.Cleanup(u.srv.Close) + return u +} + +// On routers the guard reads the per-model per-slot context: `small` serves "shared" from a +// 8192-context child with two slots (4096 per slot), `big` from a 131072-context child with one. +// The plain /props of both says nothing, so a v2 guard that only knew host-level figures would +// stay inert and let the oversized prompt overflow `small`. +func TestRouterGuardUsesPerModelContext(t *testing.T) { + small := routerUpstream(t, "small", map[string][2]int{"shared": {8192, 2}}) + big := routerUpstream(t, "big", map[string][2]int{"shared": {131072, 1}}) + r := newRig(t, ctxHosts, small, big) + + resp := r.post("/r/v1/chat/completions", bodyOfTokens(100)) + drain(resp) + if got := resp.Header.Get(proxy.HostHeader); got != "small" { + t.Fatalf("small prompt went to %q, want small (weight 10)", got) + } + resp = r.post("/r/v1/chat/completions", bodyOfTokens(6000)) + drain(resp) + if resp.StatusCode != 200 || resp.Header.Get(proxy.HostHeader) != "big" { + t.Fatalf("6000-token prompt: status %d host %q, want 200 on big", resp.StatusCode, resp.Header.Get(proxy.HostHeader)) + } + if got := resp.Header.Get("X-Crossbar-Ctx"); got != "moved:small>big" { + t.Errorf("X-Crossbar-Ctx = %q, want moved:small>big", got) + } +} + +// A model the router lists as unloaded is not resident there: the guard must not move a prompt +// to that host, and the refusal names the largest per-slot context among hosts that do serve it. +func TestRouterUnloadedModelIsNotACandidate(t *testing.T) { + small := routerUpstream(t, "small", map[string][2]int{"shared": {8192, 2}}) + big := routerUpstream(t, "big", map[string][2]int{"other": {131072, 1}}, "shared") // shared unloaded on big + r := newRig(t, ctxHosts, small, big) + + resp := r.post("/r/v1/chat/completions", bodyOfTokens(6000)) + body := drain(resp) + if resp.StatusCode != 400 { + t.Fatalf("status %d body %s, want 400: shared is loaded only on small, where it does not fit", resp.StatusCode, body) + } + if big.hits.Load() != 0 { + t.Errorf("big served %d requests for a model it does not have loaded", big.hits.Load()) + } + var e map[string]any + if err := json.Unmarshal([]byte(body), &e); err != nil { + t.Fatalf("body %q is not JSON: %v", body, err) + } + if max, _ := e["max"].(float64); max != 4096 { + t.Errorf("max = %v, want 4096: the largest per-slot context among hosts that have shared loaded", e["max"]) + } +}