From bf23b41568f39d3e81ce579f485b99fb9c791d56 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Fri, 21 Aug 2026 23:22:51 +0100 Subject: [PATCH] feat(admin): rates + discovery flag in API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend GET /admin/api/overview's existing providers payload rather than adding a sibling endpoint: the route already returns one providers array per request on the dashboard's 5s poll, is already admin-gated and stats-excluded, and the new fields are one more batched limiter read sharing that same request — a second endpoint would duplicate all three properties for no benefit. adminProviderView gains AttemptsDay/FailuresDay/AttemptsMinute/ FailuresMinute (the provider's own current-window counters) and ModelRates (a per-model breakdown, keyed by upstream model id, via the new adminModelRateView) — Feature A. buildAdminOverview builds every provider's and every (provider, model) pair's limitScope in two passes and reads them all through limiter.providerUsage in ONE storeGetMulti call, then slices the flat result back by index. adminProviderView also gains DiscoveryEnabled, mirroring a new providerSnapshot.discoveryEnabled field (registry.go) read straight off providerState — it is set once at construction and never mutated afterward, so it needs neither providerState's mutex nor a place in its existing snapshot() method's mutable-field return shape. Feature B: this lets the webui tell "discovery is off" apart from "discovery is on but has not refreshed yet", which previously both showed the same zero lastRefresh as an identical "refreshed never". --- admin.go | 107 +++++++++++++++++++++++++++---- admin_test.go | 164 +++++++++++++++++++++++++++++++++++++++++++++++ registry.go | 31 +++++++-- registry_test.go | 33 ++++++++++ 4 files changed, 317 insertions(+), 18 deletions(-) diff --git a/admin.go b/admin.go index dd666b3..d3bcea6 100644 --- a/admin.go +++ b/admin.go @@ -260,11 +260,20 @@ func sanitizeProviderErr(lastErr, rawBaseURL string) string { // userinfo/query an operator mistakenly embedded in it is stripped by // sanitizeBaseURL before this view is ever built. type adminProviderView struct { - Name string `json:"name"` - Type string `json:"type"` - BaseURL string `json:"baseUrl"` LastRefresh time.Time `json:"lastRefresh"` - LastErr string `json:"lastErr,omitempty"` + // ModelRates is this provider's per-model breakdown of the Attempts*/ + // Failures* counters below, keyed by upstream model id (Feature A, + // v0.22) — every entry in Models gets one, even a model with zero + // attempts, so the Providers tab's provider accordion can decide + // "degraded, show a badge" per model without a second round trip. + // Never nil, mirroring Models' own "never nil" convention below — + // marshals as "{}" for a provider with no known models yet, not + // omitted. + ModelRates map[string]adminModelRateView `json:"modelRates"` + Type string `json:"type"` + BaseURL string `json:"baseUrl"` + LastErr string `json:"lastErr,omitempty"` + Name string `json:"name"` // Models is the sorted explicit∪discovered model id set // (registry.go's providerSnapshot.models — provider-model-accordion // task, Vue admin panel): the dashboard's expandable provider row. @@ -273,6 +282,40 @@ type adminProviderView struct { // from a field a stale client build does not know how to read. Models []string `json:"models"` ModelCount int `json:"modelCount"` + // AttemptsDay/FailuresDay/AttemptsMinute/FailuresMinute are this + // provider's current-window upstream-attempt counters (Feature A, + // v0.22 — see limiter.recordProviderAttempt, limits.go): every + // upstream HTTP attempt increments Attempts*; only a provider-fault + // outcome (isTransient's classification, retry.go) additionally + // increments Failures*. The Providers tab's success-rate badge is + // (AttemptsDay-FailuresDay)/AttemptsDay; AttemptsMinute/FailuresMinute + // exist only for the badge's "right now" title-attribute detail — no + // day-window figure alone tells an operator whether a spike is still + // happening. Both stay 0 for a provider with no traffic yet, which the + // webui renders as a distinct "no traffic" state rather than a + // (misleadingly perfect) 100% badge. + AttemptsDay int64 `json:"attemptsDay"` + FailuresDay int64 `json:"failuresDay"` + AttemptsMinute int64 `json:"attemptsMinute"` + FailuresMinute int64 `json:"failuresMinute"` + // DiscoveryEnabled mirrors providerSnapshot.discoveryEnabled (Feature + // B, v0.22): the Providers tab uses it to tell a provider whose + // discovery is off apart from one that is on but has not completed its + // first refresh yet — both otherwise show the same zero LastRefresh. + // See registry.go's providerSnapshot.discoveryEnabled doc comment. + DiscoveryEnabled bool `json:"discoveryEnabled"` +} + +// adminModelRateView is one upstream model's current-window attempt/ +// failure counters within its provider — adminProviderView.ModelRates' +// value type (Feature A, v0.22). Field shape and meaning are identical to +// adminProviderView's own Attempts*/Failures* fields, just scoped to one +// (provider, model) pair instead of the whole provider. +type adminModelRateView struct { + AttemptsDay int64 `json:"attemptsDay"` + FailuresDay int64 `json:"failuresDay"` + AttemptsMinute int64 `json:"attemptsMinute"` + FailuresMinute int64 `json:"failuresMinute"` } // adminRedisView is the redis status line in GET /admin/api/overview. @@ -348,16 +391,58 @@ type adminOverviewResponse struct { // them. func (g *Gateway) buildAdminOverview() adminOverviewResponse { snaps := g.registry.snapshot() + + // Feature A (v0.22): ONE batched limiter read for every provider's and + // every (provider, model) pair's current attempt/failure counters — + // scopes built here in two fixed passes (every provider, then every + // provider's models, in snaps' own order), so providerUsage's flat + // result slices back by plain index: the first len(snaps) entries are + // the provider-level counters, in order; the rest are the per-model + // counters, grouped by provider in the same order and, within a + // provider, in s.models' own order — exactly how the loop below + // consumes them. + scopes := make([]limitScope, 0, len(snaps)) + for _, s := range snaps { + scopes = append(scopes, limitScope{kind: kindProvider, id: s.name}) + } + for _, s := range snaps { + for _, model := range s.models { + scopes = append(scopes, limitScope{kind: kindProviderModel, id: providerModelScopeID(s.name, model)}) + } + } + allCounters := g.limiter.providerUsage(scopes) + providerLevelCounters := allCounters[:len(snaps)] + modelCounters := allCounters[len(snaps):] + providers := make([]adminProviderView, len(snaps)) + mi := 0 for i, s := range snaps { + pc := providerLevelCounters[i] + modelRates := make(map[string]adminModelRateView, len(s.models)) + for _, model := range s.models { + mc := modelCounters[mi] + mi++ + modelRates[model] = adminModelRateView{ + AttemptsDay: mc.attemptsDay, + FailuresDay: mc.failuresDay, + AttemptsMinute: mc.attemptsMinute, + FailuresMinute: mc.failuresMinute, + } + } providers[i] = adminProviderView{ - Name: s.name, - Type: s.typeName, - BaseURL: sanitizeBaseURL(s.baseURL), - Models: s.models, - ModelCount: s.modelCount, - LastRefresh: s.lastRefresh, - LastErr: sanitizeProviderErr(s.lastErr, s.baseURL), + Name: s.name, + Type: s.typeName, + BaseURL: sanitizeBaseURL(s.baseURL), + Models: s.models, + ModelCount: s.modelCount, + LastRefresh: s.lastRefresh, + LastErr: sanitizeProviderErr(s.lastErr, s.baseURL), + DiscoveryEnabled: s.discoveryEnabled, + AttemptsDay: pc.attemptsDay, + FailuresDay: pc.failuresDay, + AttemptsMinute: pc.attemptsMinute, + FailuresMinute: pc.failuresMinute, + ModelRates: modelRates, } } diff --git a/admin_test.go b/admin_test.go index 0401ddb..39fb3e8 100644 --- a/admin_test.go +++ b/admin_test.go @@ -12,6 +12,7 @@ import ( "regexp" "slices" "strings" + "sync/atomic" "testing" "time" ) @@ -378,6 +379,169 @@ func TestAdminOverview_ProviderLastErrAfterFailedRefresh(t *testing.T) { } } +// --- overview: Feature A (v0.22) per-provider/per-model success-rate accounting --- + +// TestAdminOverview_ProviderRates_NoTraffic_AllZeroWithModelRatesPopulated +// proves a provider with no traffic yet reports every Attempts*/Failures* +// field at 0 (the webui's "no traffic" state, not a misleadingly perfect +// 100%), and ModelRates carries one zero-valued entry per configured +// model — never an empty/omitted map, mirroring Models' own convention. +func TestAdminOverview_ProviderRates_NoTraffic_AllZeroWithModelRatesPopulated(t *testing.T) { + t.Parallel() + cfg := newAdminTestConfig() + h, _ := newAdminGatewayHandle(t, cfg) + + rec := httptest.NewRecorder() + h.ServeHTTP(rec, adminRequest(http.MethodGet, adminOverviewPath, "sk-admin1")) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String()) + } + var got adminOverviewResponse + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("decode: %v", err) + } + + for _, p := range got.Providers { + if p.AttemptsDay != 0 || p.FailuresDay != 0 || p.AttemptsMinute != 0 || p.FailuresMinute != 0 { + t.Errorf("provider %q rates = %+v, want all-zero (no traffic yet)", p.Name, p) + } + if p.ModelRates == nil { + t.Errorf("provider %q ModelRates is nil, want a non-nil map (marshals as {})", p.Name) + } + for _, model := range p.Models { + mr, ok := p.ModelRates[model] + if !ok { + t.Errorf("provider %q ModelRates missing entry for model %q", p.Name, model) + continue + } + if mr != (adminModelRateView{}) { + t.Errorf("provider %q model %q rates = %+v, want all-zero", p.Name, model, mr) + } + } + } +} + +// TestAdminOverview_DiscoveryEnabledFlag proves adminProviderView. +// DiscoveryEnabled mirrors each provider's own configured Discovery flag — +// Feature B (v0.22): the webui needs this to tell "discovery is off" apart +// from "discovery is on but has not refreshed yet", both of which +// otherwise show the same zero LastRefresh. +func TestAdminOverview_DiscoveryEnabledFlag(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"data":[]}`)) + })) + defer srv.Close() + + cfg := CreateConfig() + cfg.Admin = &AdminConfig{Enabled: true} + cfg.Providers = map[string]*ProviderConfig{ + "discoverable": {Type: "openai", BaseURL: srv.URL, APIKey: "sk", Discovery: true}, + "pinned": {Type: "openai", BaseURL: srv.URL, APIKey: "sk", Models: []string{"pinned-model"}}, + } + cfg.Groups = map[string]*GroupConfig{"g": {}} + cfg.Users = &UsersConfig{Inline: []*UserConfig{{Name: "admin1", Group: "g", APIKey: "sk-admin1", Admin: true}}} + h, _ := newAdminGatewayHandle(t, cfg) + + rec := httptest.NewRecorder() + h.ServeHTTP(rec, adminRequest(http.MethodGet, adminOverviewPath, "sk-admin1")) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String()) + } + var got adminOverviewResponse + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("decode: %v", err) + } + if len(got.Providers) != 2 { + t.Fatalf("providers = %+v, want 2 entries", got.Providers) + } + for _, p := range got.Providers { + switch p.Name { + case "discoverable": + if !p.DiscoveryEnabled { + t.Errorf("provider %q discoveryEnabled = false, want true", p.Name) + } + case "pinned": + if p.DiscoveryEnabled { + t.Errorf("provider %q discoveryEnabled = true, want false", p.Name) + } + default: + t.Fatalf("unexpected provider %q", p.Name) + } + } +} + +// TestAdminOverview_ProviderRates_EndToEndAfterTraffic drives one +// successful and one provider-fault (500) chat completion through the +// SAME (provider, model), then asserts GET /admin/api/overview reports +// exactly 2 attempts and 1 failure at both provider and model scope — end +// to end from runUnified's attemptRecorder wiring (routes_unified.go) +// through retryPolicy.do (retry.go) to recordProviderAttempt (limits.go) +// to buildAdminOverview's own batched read (admin.go). +func TestAdminOverview_ProviderRates_EndToEndAfterTraffic(t *testing.T) { + t.Parallel() + var callN int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if atomic.AddInt32(&callN, 1) == 1 { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"id":"c1","object":"chat.completion","model":"gpt-test","choices":[{"index":0,"message":{"role":"assistant","content":"hi"},"finish_reason":"stop"}],"usage":{"prompt_tokens":1,"completion_tokens":1}}`)) + return + } + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":"boom"}`)) + })) + defer srv.Close() + + cfg := CreateConfig() + cfg.Admin = &AdminConfig{Enabled: true} + cfg.Providers = map[string]*ProviderConfig{ + "openai": {Type: "openai", BaseURL: srv.URL, APIKey: "sk-up", Models: []string{"gpt-test"}}, + } + cfg.Groups = map[string]*GroupConfig{"default": {}} + cfg.Users = &UsersConfig{Inline: []*UserConfig{ + {Name: "alice", Group: "default", APIKey: "sk-alice", Limits: &LimitsConfig{}}, + {Name: "admin1", Group: "default", APIKey: "sk-admin1", Admin: true}, + }} + h, _ := newAdminGatewayHandle(t, cfg) + + body := map[string]any{"model": "gpt-test", "messages": []any{map[string]any{"role": "user", "content": "hi"}}} + for i := 0; i < 2; i++ { + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + } + + rec := httptest.NewRecorder() + h.ServeHTTP(rec, adminRequest(http.MethodGet, adminOverviewPath, "sk-admin1")) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String()) + } + var got adminOverviewResponse + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("decode: %v", err) + } + if len(got.Providers) != 1 { + t.Fatalf("providers = %+v, want 1 entry", got.Providers) + } + p := got.Providers[0] + if p.AttemptsDay != 2 || p.FailuresDay != 1 { + t.Errorf("provider attempts/failures = %d/%d, want 2/1", p.AttemptsDay, p.FailuresDay) + } + if p.AttemptsMinute != 2 || p.FailuresMinute != 1 { + t.Errorf("provider attempts/failures (minute) = %d/%d, want 2/1", p.AttemptsMinute, p.FailuresMinute) + } + mr, ok := p.ModelRates["gpt-test"] + if !ok { + t.Fatalf("modelRates missing entry for gpt-test: %+v", p.ModelRates) + } + if mr.AttemptsDay != 2 || mr.FailuresDay != 1 { + t.Errorf("model attempts/failures = %d/%d, want 2/1", mr.AttemptsDay, mr.FailuresDay) + } +} + // --- usage: math against seeded counters --- func TestAdminUsage_MathAgainstSeededCounters(t *testing.T) { diff --git a/registry.go b/registry.go index 271acdb..55f1c22 100644 --- a/registry.go +++ b/registry.go @@ -778,6 +778,22 @@ type providerSnapshot struct { lastErr string models []string modelCount int + // discoveryEnabled mirrors providerState.discoveryEnabled (Feature B, + // v0.22 last-refresh label honesty): read directly rather than through + // providerState.snapshot(), because it is set once at construction + // (newModelRegistry, from ProviderConfig.Discovery) and never + // reassigned afterward — every other providerState field warmFill/ + // maybeRefresh/finishRefresh mutate only reads it, never writes it — so + // it needs neither the mutex snapshot() takes for the fields that DO + // mutate + // (models/lastRefresh/lastErr) nor a place in that method's return + // shape. The admin dashboard needs it to tell "discovery is off" apart + // from "discovery is on but has not refreshed yet" — both currently + // read identically as a zero LastRefresh, which GET /admin/api/overview + // (admin.go) and the Providers tab (webui) previously rendered as the + // misleading "refreshed never" for a provider that will never refresh + // by design. + discoveryEnabled bool } // aliasSnapshotEntry is one configured alias's read-only view for the @@ -822,13 +838,14 @@ func (m *modelRegistry) snapshot() []providerSnapshot { // that class of trap entirely. models, lastRefresh, lastErr := m.states[name].snapshot() out = append(out, providerSnapshot{ - name: adapter.name(), - typeName: adapter.typeName(), - baseURL: adapter.base(), - models: models, - modelCount: len(models), - lastRefresh: lastRefresh, - lastErr: lastErr, + name: adapter.name(), + typeName: adapter.typeName(), + baseURL: adapter.base(), + models: models, + modelCount: len(models), + lastRefresh: lastRefresh, + lastErr: lastErr, + discoveryEnabled: m.states[name].discoveryEnabled, }) } return out diff --git a/registry_test.go b/registry_test.go index bb0ae10..a80a194 100644 --- a/registry_test.go +++ b/registry_test.go @@ -707,6 +707,39 @@ func TestModelRegistry_Snapshot_ModelsStaleWhileError(t *testing.T) { } } +// TestModelRegistry_Snapshot_DiscoveryEnabledReflectsConfig proves +// providerSnapshot.discoveryEnabled (Feature B, v0.22 — last-refresh label +// honesty) mirrors each provider's own configured Discovery flag exactly, +// read straight from providerState rather than through providerState. +// snapshot()'s mutex-guarded fields, since it never changes after +// construction. +func TestModelRegistry_Snapshot_DiscoveryEnabledReflectsConfig(t *testing.T) { + t.Parallel() + adapters := map[string]providerAdapter{ + "on": newFakeAdapter("on"), + "off": newFakeAdapter("off"), + } + cfg := &Config{Providers: map[string]*ProviderConfig{ + "on": {Discovery: true, DiscoveryInterval: "1h"}, + "off": {Models: []string{"pinned"}}, + }} + reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {}) + if err != nil { + t.Fatalf("newModelRegistry: %v", err) + } + + byName := map[string]providerSnapshot{} + for _, s := range reg.snapshot() { + byName[s.name] = s + } + if !byName["on"].discoveryEnabled { + t.Error(`provider "on" discoveryEnabled = false, want true`) + } + if byName["off"].discoveryEnabled { + t.Error(`provider "off" discoveryEnabled = true, want false`) + } +} + // --- listFor: group filtering, sorting, collision presentation (ruling h) --- func TestModelRegistry_ListFor_GroupFiltered(t *testing.T) {