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) {