diff --git a/internal/router/handlers.go b/internal/router/handlers.go index 366f1ffa..4e859c37 100644 --- a/internal/router/handlers.go +++ b/internal/router/handlers.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "io" "net/http" "slices" @@ -107,6 +108,9 @@ func (r *Router) Handler() http.Handler { // authenticated by a session cookie, not by an API key, which is why // they are exempt below. pages := web.NewAccountPages(r.accounts, r.store, r.store, version.Version, r.log) + // So the example on the profile page names a model that is actually + // running, rather than a placeholder that gets sent as-is. + pages.ServedModels(r.exampleModels) pages.Register(mux) r.registerChat(mux, pages) @@ -460,6 +464,51 @@ func injectIncludeUsage(body []byte) ([]byte, bool) { return out, true } +// noEndpointFor explains that nothing serves a model, and says what does. +// +// "no endpoint serves \"MODEL\"" was all a client got, which is true and +// useless: it reads the same whether the name was a typo, a placeholder left +// in from the documentation, or a real outage. Someone copying the curl from +// the account page sent the literal word MODEL and had nothing to go on. +// +// The list is what /v1/models already returns to the same caller, so naming it +// here discloses nothing new. It is capped because a large catalog would turn +// an error message into a page. +func noEndpointFor(model string, served []string) string { + if len(served) == 0 { + return fmt.Sprintf("no endpoint serves %q, and no model is currently being served", model) + } + const showAt = 10 + shown := served + suffix := "" + if len(shown) > showAt { + shown, suffix = shown[:showAt], fmt.Sprintf(" and %d more", len(served)-showAt) + } + return fmt.Sprintf("no endpoint serves %q; available models are %s%s", + model, strings.Join(shown, ", "), suffix) +} + +// exampleModels is servedModels with the ones that answer for real first. +// +// The profile page takes the first name for its copy-pasteable example, and +// handing it a simulated model would send someone's first request to an engine +// that replies in Greek letters. +func (r *Router) exampleModels() []string { + names := r.servedModels() + slices.SortStableFunc(names, func(a, b string) int { + as, bs := r.models[a].Simulated(), r.models[b].Simulated() + switch { + case as == bs: + return 0 + case bs: + return -1 + default: + return 1 + } + }) + return names +} + // servedModels lists the canonical models at least one endpoint serves. func (r *Router) servedModels() []string { seen := map[string]struct{}{} diff --git a/internal/router/router.go b/internal/router/router.go index 2a09dbf6..b879ded3 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -419,7 +419,7 @@ var errNoCapacity = errors.New("no endpoint available") func (r *Router) dispatchOne(ctx context.Context, w http.ResponseWriter, req *parsedRequest, model string, entry catalog.Model) dispatchResult { candidates := r.reg.Snapshots(model) if len(candidates) == 0 { - return dispatchResult{err: fmt.Errorf("%w: no endpoint serves %q", errNoCapacity, model)} + return dispatchResult{err: fmt.Errorf("%w: %s", errNoCapacity, noEndpointFor(model, r.servedModels()))} } profile := r.profileFor(entry, req.ProfileOverride) diff --git a/internal/router/router_test.go b/internal/router/router_test.go index c839f687..8627a505 100644 --- a/internal/router/router_test.go +++ b/internal/router/router_test.go @@ -363,3 +363,55 @@ func TestSessionHeaderFallsBackToUser(t *testing.T) { t.Errorf("the header should win, got %q", got) } } + +func TestUnknownModelErrorNamesWhatIsAvailable(t *testing.T) { + // Reported by a user who copied the curl from the account page and sent + // its placeholder unchanged. "no endpoint serves \"MODEL\"" is true and + // useless: it reads the same for a typo, a placeholder and a real outage. + e := newFakeEngine(t, "m") + rt := newTestRouter(t, baseConfig(config.Endpoint{ID: "a", URL: e.srv.URL, Models: []string{"m"}})) + + rec := post(t, rt.Handler(), map[string]any{"model": "MODEL", "messages": []any{}}, nil) + if rec.Code != http.StatusServiceUnavailable { + t.Fatalf("status %d, expected 503", rec.Code) + } + var body struct { + Error struct { + Message string `json:"message"` + } `json:"error"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("error body is not JSON: %q", rec.Body.String()) + } + if !strings.Contains(body.Error.Message, `"MODEL"`) { + t.Errorf("the message does not repeat what was asked for: %q", body.Error.Message) + } + if !strings.Contains(body.Error.Message, "m") || + !strings.Contains(body.Error.Message, "available") { + t.Errorf("the message does not say what is available: %q", body.Error.Message) + } +} + +func TestTheErrorDoesNotBecomeAPage(t *testing.T) { + // A large catalog must not turn one error into a wall of names. + var many []string + for i := range 40 { + many = append(many, fmt.Sprintf("model-%02d", i)) + } + msg := noEndpointFor("nope", many) + if !strings.Contains(msg, "and 30 more") { + t.Errorf("the list was not capped: %q", msg) + } + if strings.Contains(msg, "model-11") { + t.Errorf("more than the cap was listed: %q", msg) + } +} + +func TestTheErrorIsHonestWhenNothingIsServed(t *testing.T) { + // Listing nothing as "available models are " would read as a truncated + // message rather than an empty cluster. + msg := noEndpointFor("m", nil) + if !strings.Contains(msg, "no model is currently being served") { + t.Errorf("an empty fleet is not explained: %q", msg) + } +} diff --git a/internal/web/account.go b/internal/web/account.go index d9cfff43..5439e33f 100644 --- a/internal/web/account.go +++ b/internal/web/account.go @@ -47,6 +47,33 @@ type AccountPages struct { passwords account.PasswordChanger version string log *slog.Logger + // served, when set, names the models an endpoint is currently serving. The + // curl example on this page fills its model field from it: every other + // field in that snippet is real, so the one placeholder left in it was + // copied verbatim and sent as the literal string MODEL. + served func() []string +} + +// ServedModels supplies the names the profile page's example should use. +// +// A setter rather than a constructor argument because only the router knows +// what is being served, and the page is useful without it — with no source the +// example falls back to a placeholder, which is what it always was. +func (p *AccountPages) ServedModels(f func() []string) { p.served = f } + +// exampleModel picks the model the copy-pasteable example should name. +// +// A real model over a simulated one is not expressible here — this package does +// not know which is which — so it takes the first served name and relies on the +// router listing real models first. +func (p *AccountPages) exampleModel() string { + if p.served == nil { + return "" + } + if names := p.served(); len(names) > 0 { + return names[0] + } + return "" } // NewAccountPages builds the handlers. @@ -189,8 +216,11 @@ type profileData struct { CSRF string BaseURL string - Version string - Now string + // ExampleModel is a model that is actually being served, so the example on + // the page can be copied and run as it stands. + ExampleModel string + Version string + Now string } func (p *AccountPages) profile(w http.ResponseWriter, r *http.Request) { @@ -272,6 +302,7 @@ func (p *AccountPages) render(w http.ResponseWriter, r *http.Request, d.User = user d.MinPassword = account.MinPasswordLen d.BaseURL = baseURL(r) + d.ExampleModel = p.exampleModel() d.Version = p.version d.Now = time.Now().UTC().Format(time.RFC3339) // The form token comes from the cookie, never from whatever the request @@ -474,6 +505,7 @@ func (p *AccountPages) renderWithCSRF(w http.ResponseWriter, r *http.Request, d.User = user d.MinPassword = account.MinPasswordLen d.BaseURL = baseURL(r) + d.ExampleModel = p.exampleModel() d.Version = p.version d.Now = time.Now().UTC().Format(time.RFC3339) d.CSRF = csrf diff --git a/internal/web/account_test.go b/internal/web/account_test.go new file mode 100644 index 00000000..3cd3201a --- /dev/null +++ b/internal/web/account_test.go @@ -0,0 +1,21 @@ +package web + +import "testing" + +func TestTheExampleNamesARealModel(t *testing.T) { + // Every other field in that curl is filled in — the base URL, the key — + // so the one placeholder in it reads like a value. It was copied verbatim + // and sent as the literal string MODEL. + p := &AccountPages{} + if got := p.exampleModel(); got != "" { + t.Errorf("with no source the example should be an obvious placeholder, got %q", got) + } + p.ServedModels(func() []string { return nil }) + if got := p.exampleModel(); got != "" { + t.Errorf("with nothing served the example should be a placeholder, got %q", got) + } + p.ServedModels(func() []string { return []string{"qwen", "big-model"} }) + if got := p.exampleModel(); got != "qwen" { + t.Errorf("example model = %q, want the first served name", got) + } +} diff --git a/internal/web/templates/account.html b/internal/web/templates/account.html index 4b971186..b095b6b8 100644 --- a/internal/web/templates/account.html +++ b/internal/web/templates/account.html @@ -73,7 +73,7 @@ Tell whoever you asked for access that you have signed up.

curl {{.BaseURL}}/v1/chat/completions \
   -H "Authorization: Bearer {{.NewKey}}" \
   -H "Content-Type: application/json" \
-  -d '{"model": "MODEL", "messages": [{"role":"user","content":"hello"}]}'
+ -d '{"model": "{{.ExampleModel}}", "messages": [{"role":"user","content":"hello"}]}' {{end}}

Create a key

diff --git a/internal/web/templates/claim.html b/internal/web/templates/claim.html index 834dfb31..e88fce36 100644 --- a/internal/web/templates/claim.html +++ b/internal/web/templates/claim.html @@ -20,7 +20,12 @@ curl {{.BaseURL}}/v1/chat/completions \ -H "Authorization: Bearer $LLMGW_API_KEY" \ -H "Content-Type: application/json" \ - -d '{"model": "MODEL", "messages": [{"role":"user","content":"hello"}]}' + -d '{"model": "<model-name>", "messages": [{"role":"user","content":"hello"}]}' +

+ Replace <model-name> with a name from + GET {{.BaseURL}}/v1/models, which needs your key. Sending the + placeholder unchanged is answered with the list of models that are available. +

from openai import OpenAI
 client = OpenAI(base_url="{{.BaseURL}}/v1", api_key=os.environ["LLMGW_API_KEY"])

GET {{.BaseURL}}/v1/models lists what this key may use. diff --git a/internal/web/templates/landing.html b/internal/web/templates/landing.html index 02eba43a..9280c6be 100644 --- a/internal/web/templates/landing.html +++ b/internal/web/templates/landing.html @@ -18,7 +18,12 @@ client for; only the base URL changes.

curl {{.BaseURL}}/v1/chat/completions \
   -H "Authorization: Bearer $LLMGW_API_KEY" \
   -H "Content-Type: application/json" \
-  -d '{"model": "MODEL", "messages": [{"role":"user","content":"hello"}]}'
+ -d '{"model": "<model-name>", "messages": [{"role":"user","content":"hello"}]}' +

+ Replace <model-name> with a name from + GET {{.BaseURL}}/v1/models, which needs your key. Sending the + placeholder unchanged is answered with the list of models that are available. +

from openai import OpenAI
 client = OpenAI(base_url="{{.BaseURL}}/v1", api_key=os.environ["LLMGW_API_KEY"])