Remove superseded configuration loaders and consolidate runtime tests

internal/backend/registry.go

@@ -5,31 +5,21 @@ package backend
 import (
 	"errors"
 	"fmt"
-	"regexp"
-	"sort"
-	"strings"
 
 	"gitea.maximumdirect.net/eric/promptkit/internal/domain"
 	"gitea.maximumdirect.net/eric/promptkit/internal/jsonvalue"
-	"gitea.maximumdirect.net/eric/promptkit/internal/llm"
 )
 
 const (
-	// OpenRouterID is the reserved ID of Promptkit's built-in OpenRouter
-	// backend.
+	// OpenRouterID identifies the maintained OpenRouter backend.
 	OpenRouterID = "openrouter"
-	// RakestrawHomeID is the reserved ID of Promptkit's built-in Rakestrawhome
-	// backend.
+	// RakestrawHomeID identifies the maintained Rakestrawhome backend.
 	RakestrawHomeID = "rakestrawhome"
-
-	defaultQueueCapacity = 1024
 )
 
 // ErrBackendNotFound identifies a registry lookup for an unknown backend ID.
 var ErrBackendNotFound = errors.New("backend not found")
 
-var environmentVariableName = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`)
-
 // Registry is an immutable collection of validated backend definitions.
 type Registry struct {
 	backends map[string]domain.Backend
@@ -52,36 +42,6 @@ func NewEffectiveRegistry(definitions map[string]domain.Backend) (*Registry, err
 	return registry, nil
 }
 
-// NewRegistry constructs a registry containing maintained definitions followed
-// by consumer additions. Every ID must be unique across both groups.
-func NewRegistry(maintained, additions []domain.Backend) (*Registry, error) {
-	registry := &Registry{
-		backends: make(map[string]domain.Backend, len(maintained)+len(additions)),
-	}
-
-	definitions := make([]domain.Backend, 0, len(maintained)+len(additions))
-	definitions = append(definitions, maintained...)
-	definitions = append(definitions, additions...)
-
-	for _, definition := range definitions {
-		definition.ID = strings.TrimSpace(definition.ID)
-		if definition.ID == "" {
-			return nil, errors.New("backend ID must not be blank")
-		}
-		if _, exists := registry.backends[definition.ID]; exists {
-			return nil, fmt.Errorf("backend ID %q is already registered", definition.ID)
-		}
-
-		normalized, err := NormalizeDefinition(definition)
-		if err != nil {
-			return nil, err
-		}
-		registry.backends[normalized.ID] = normalized
-	}
-
-	return registry, nil
-}
-
 // GetBackend returns a defensive copy of the backend registered with id.
 func (r *Registry) GetBackend(id string) (domain.Backend, error) {
 	if r == nil {
@@ -117,80 +77,3 @@ func (r *Registry) CapacityPolicies() map[string]domain.BackendCapacityPolicy {
 	}
 	return policies
 }
-
-// NormalizeDefinition validates and defensively copies one backend definition.
-func NormalizeDefinition(definition domain.Backend) (domain.Backend, error) {
-	endpoint, err := domain.NormalizeOpenAICompatibleBaseEndpoint(definition.Endpoint)
-	if err != nil {
-		return domain.Backend{}, fmt.Errorf("backend %q endpoint: %w", definition.ID, err)
-	}
-	definition.Endpoint = endpoint
-
-	definition.APIKeyEnv = strings.TrimSpace(definition.APIKeyEnv)
-	if definition.APIKeyEnv != "" && !environmentVariableName.MatchString(definition.APIKeyEnv) {
-		return domain.Backend{}, fmt.Errorf(
-			"backend %q api key environment variable %q is invalid",
-			definition.ID,
-			definition.APIKeyEnv,
-		)
-	}
-
-	if definition.ConcurrencyLimit < 0 {
-		return domain.Backend{}, fmt.Errorf(
-			"backend %q concurrency limit must not be negative",
-			definition.ID,
-		)
-	}
-	if definition.QueueCapacity < 0 {
-		return domain.Backend{}, fmt.Errorf(
-			"backend %q queue capacity must not be negative",
-			definition.ID,
-		)
-	}
-	if definition.ConcurrencyLimit == 0 {
-		if definition.QueueCapacitySet {
-			return domain.Backend{}, fmt.Errorf(
-				"backend %q queue capacity requires a positive concurrency limit",
-				definition.ID,
-			)
-		}
-		definition.QueueCapacity = 0
-	} else {
-		if !definition.QueueCapacitySet {
-			definition.QueueCapacity = defaultQueueCapacity
-			definition.QueueCapacitySet = true
-		}
-		maxInt := int(^uint(0) >> 1)
-		if definition.QueueCapacity > maxInt-definition.ConcurrencyLimit {
-			return domain.Backend{}, fmt.Errorf(
-				"backend %q total capacity overflows int",
-				definition.ID,
-			)
-		}
-	}
-
-	keys := make([]string, 0, len(definition.ExtraParams))
-	for key := range definition.ExtraParams {
-		keys = append(keys, key)
-	}
-	sort.Strings(keys)
-	for _, key := range keys {
-		if key == "" {
-			return domain.Backend{}, fmt.Errorf("backend %q extra parameter key must not be empty", definition.ID)
-		}
-		if llm.IsReservedOpenAIChatRequestField(key) {
-			return domain.Backend{}, fmt.Errorf(
-				"backend %q extra parameter %q collides with a reserved request field",
-				definition.ID,
-				key,
-			)
-		}
-	}
-
-	extraParams, err := jsonvalue.CopyMap(definition.ExtraParams)
-	if err != nil {
-		return domain.Backend{}, fmt.Errorf("backend %q extra parameters: %w", definition.ID, err)
-	}
-	definition.ExtraParams = extraParams
-	return definition, nil
-}

internal/backend/registry_test.go

@@ -3,55 +3,19 @@ package backend_test
 import (
 	"errors"
 	"reflect"
-	"strings"
 	"testing"
 
 	"gitea.maximumdirect.net/eric/promptkit/internal/backend"
 	"gitea.maximumdirect.net/eric/promptkit/internal/domain"
 )
 
-const validEndpoint = "https://backend.example/v1"
-
-func TestRegistryIncludesMaintainedDefinitions(t *testing.T) {
-	maintained := []domain.Backend{
-		{ID: backend.OpenRouterID, Endpoint: validEndpoint, ConcurrencyLimit: 2, QueueCapacity: 3, QueueCapacitySet: true},
-		{ID: backend.RakestrawHomeID, Endpoint: "https://second.example/v1", ConcurrencyLimit: 4, QueueCapacity: 5, QueueCapacitySet: true},
-	}
-	registry, err := backend.NewRegistry(maintained, nil)
-	if err != nil {
-		t.Fatalf("construct registry: %v", err)
-	}
-
-	for _, expected := range maintained {
-		t.Run(expected.ID, func(t *testing.T) {
-			definition, err := registry.GetBackend(expected.ID)
-			if err != nil {
-				t.Fatalf("look up maintained definition: %v", err)
-			}
-			if !reflect.DeepEqual(definition, expected) {
-				t.Fatalf("unexpected maintained definition: %#v", definition)
-			}
-		})
-	}
-
-	policies := registry.CapacityPolicies()
-	if len(policies) != 2 ||
-		policies[backend.OpenRouterID] != (domain.BackendCapacityPolicy{
-			ConcurrencyLimit: 2,
-			QueueCapacity:    3,
-		}) ||
-		policies[backend.RakestrawHomeID] != (domain.BackendCapacityPolicy{
-			ConcurrencyLimit: 4,
-			QueueCapacity:    5,
-		}) {
-		t.Fatalf("unexpected built-in capacity policies: %#v", policies)
-	}
-}
-
-func TestEffectiveRegistryKeepsLiteralIDsAndOwnsValues(t *testing.T) {
+func TestEffectiveRegistryOwnsDefinitionsAndCapacityPolicies(t *testing.T) {
 	params := map[string]any{"nested": map[string]any{"enabled": true}}
 	registry, err := backend.NewEffectiveRegistry(map[string]domain.Backend{
-		" local.v1 ": {Endpoint: validEndpoint, ExtraParams: params, ConcurrencyLimit: 2, QueueCapacitySet: true},
+		" local.v1 ": {
+			Endpoint: "https://backend.example/v1", ExtraParams: params,
+			ConcurrencyLimit: 2, QueueCapacity: 3, QueueCapacitySet: true,
+		},
 	})
 	if err != nil {
 		t.Fatal(err)
@@ -59,341 +23,34 @@ func TestEffectiveRegistryKeepsLiteralIDsAndOwnsValues(t *testing.T) {
 	params["nested"].(map[string]any)["enabled"] = false
 	selected, err := registry.GetBackend(" local.v1 ")
 	if err != nil || selected.ID != " local.v1 " || selected.ExtraParams["nested"].(map[string]any)["enabled"] != true {
-		t.Fatalf("effective registry changed identity or retained caller values: %+v, %v", selected, err)
-	}
-	if _, err := registry.GetBackend("local.v1"); !errors.Is(err, backend.ErrBackendNotFound) {
-		t.Fatalf("trimmed identity unexpectedly matched: %v", err)
+		t.Fatalf("effective backend was not captured under its literal ID: %+v, %v", selected, err)
 	}
 	selected.ExtraParams["nested"].(map[string]any)["enabled"] = false
-	again, _ := registry.GetBackend(" local.v1 ")
-	if again.ExtraParams["nested"].(map[string]any)["enabled"] != true {
-		t.Fatal("registry lookup exposed its stored values")
+	again, err := registry.GetBackend(" local.v1 ")
+	if err != nil || again.ExtraParams["nested"].(map[string]any)["enabled"] != true {
+		t.Fatalf("lookup exposed registry values: %+v, %v", again, err)
 	}
-}
-
-func TestRegistryNormalizesUniqueAdditionsAndIsolatesMutations(t *testing.T) {
-	nested := map[string]int{"limit": 2}
-	extraParams := map[string]any{
-		"count":  int64(7),
-		"nested": nested,
-	}
-	registry, err := backend.NewRegistry(nil, []domain.Backend{
-		{
-			ID:               " custom ",
-			Endpoint:         " https://custom.example/openai/v1 ",
-			APIKeyEnv:        " CUSTOM_API_KEY ",
-			ExtraParams:      extraParams,
-			ConcurrencyLimit: 3,
-			QueueCapacity:    2,
-			QueueCapacitySet: true,
-		},
-		{
-			ID:       "Custom",
-			Endpoint: validEndpoint,
-		},
-	})
-	if err != nil {
-		t.Fatalf("construct registry: %v", err)
-	}
-
-	nested["limit"] = 99
-	extraParams["added"] = true
-
-	got, err := registry.GetBackend("custom")
-	if err != nil {
-		t.Fatalf("look up custom backend: %v", err)
-	}
-	if got.ID != "custom" ||
-		got.Endpoint != "https://custom.example/openai/v1" ||
-		got.APIKeyEnv != "CUSTOM_API_KEY" ||
-		got.ConcurrencyLimit != 3 ||
-		got.QueueCapacity != 2 ||
-		!got.QueueCapacitySet {
-		t.Fatalf("unexpected normalized definition: %#v", got)
-	}
-	if count, ok := got.ExtraParams["count"].(int64); !ok || count != 7 {
-		t.Fatalf("integer type or value changed: %#v", got.ExtraParams["count"])
-	}
-	gotNested, ok := got.ExtraParams["nested"].(map[string]int)
-	if !ok || gotNested["limit"] != 2 {
-		t.Fatalf("container type or value changed: %#v", got.ExtraParams["nested"])
-	}
-	if _, exists := got.ExtraParams["added"]; exists {
-		t.Fatalf("registry retained caller map: %#v", got.ExtraParams)
-	}
-
-	gotNested["limit"] = 100
-	got.ExtraParams["added"] = true
-	again, err := registry.GetBackend("custom")
-	if err != nil {
-		t.Fatalf("look up custom backend again: %v", err)
-	}
-	if again.ExtraParams["nested"].(map[string]int)["limit"] != 2 {
-		t.Fatalf("lookup exposed registry nested map: %#v", again.ExtraParams)
-	}
-	if _, exists := again.ExtraParams["added"]; exists {
-		t.Fatalf("lookup exposed registry map: %#v", again.ExtraParams)
-	}
-
-	if _, err := registry.GetBackend("Custom"); err != nil {
-		t.Fatalf("backend IDs should be case-sensitive: %v", err)
-	}
-
 	policies := registry.CapacityPolicies()
-	if len(policies) != 1 {
-		t.Fatalf("unexpected capacity policy count: %#v", policies)
-	}
-	policies["custom"] = domain.BackendCapacityPolicy{}
-	delete(policies, "custom")
-	againPolicies := registry.CapacityPolicies()
-	if againPolicies["custom"] != (domain.BackendCapacityPolicy{
-		ConcurrencyLimit: 3,
-		QueueCapacity:    2,
+	if !reflect.DeepEqual(policies, map[string]domain.BackendCapacityPolicy{
+		" local.v1 ": {ConcurrencyLimit: 2, QueueCapacity: 3},
 	}) {
-		t.Fatalf("capacity policy map mutated registry state: %#v", againPolicies)
+		t.Fatalf("capacity policy = %+v", policies)
+	}
+	delete(policies, " local.v1 ")
+	if len(registry.CapacityPolicies()) != 1 {
+		t.Fatal("capacity policy map exposed registry state")
 	}
 }
 
-func TestNewRegistryNormalizesCapacityPolicy(t *testing.T) {
-	maxInt := int(^uint(0) >> 1)
-	tests := []struct {
-		name       string
-		definition domain.Backend
-		want       domain.BackendCapacityPolicy
-		wantSet    bool
-		wantError  bool
-	}{
-		{
-			name:       "unlimited when omitted",
-			definition: domain.Backend{},
-		},
-		{
-			name: "default queue",
-			definition: domain.Backend{
-				ConcurrencyLimit: 2,
-			},
-			want: domain.BackendCapacityPolicy{
-				ConcurrencyLimit: 2,
-				QueueCapacity:    1024,
-			},
-			wantSet: true,
-		},
-		{
-			name: "explicit zero queue",
-			definition: domain.Backend{
-				ConcurrencyLimit: 2,
-				QueueCapacitySet: true,
-			},
-			want: domain.BackendCapacityPolicy{
-				ConcurrencyLimit: 2,
-			},
-			wantSet: true,
-		},
-		{
-			name: "negative concurrency limit",
-			definition: domain.Backend{
-				ConcurrencyLimit: -1,
-			},
-			wantError: true,
-		},
-		{
-			name: "negative queue capacity",
-			definition: domain.Backend{
-				ConcurrencyLimit: 1,
-				QueueCapacity:    -1,
-				QueueCapacitySet: true,
-			},
-			wantError: true,
-		},
-		{
-			name: "queue without limit",
-			definition: domain.Backend{
-				QueueCapacitySet: true,
-			},
-			wantError: true,
-		},
-		{
-			name: "total overflow",
-			definition: domain.Backend{
-				ConcurrencyLimit: maxInt,
-				QueueCapacity:    1,
-				QueueCapacitySet: true,
-			},
-			wantError: true,
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			tc.definition.ID = "custom"
-			tc.definition.Endpoint = validEndpoint
-			registry, err := backend.NewRegistry(nil, []domain.Backend{tc.definition})
-			if tc.wantError {
-				if err == nil {
-					t.Fatal("expected invalid capacity policy error")
-				}
-				return
-			}
-			if err != nil {
-				t.Fatalf("construct registry: %v", err)
-			}
-
-			definition, err := registry.GetBackend("custom")
-			if err != nil {
-				t.Fatalf("look up custom backend: %v", err)
-			}
-			if definition.ConcurrencyLimit != tc.want.ConcurrencyLimit ||
-				definition.QueueCapacity != tc.want.QueueCapacity ||
-				definition.QueueCapacitySet != tc.wantSet {
-				t.Fatalf("normalized capacity=(%d, %d, %t), want (%d, %d, %t)",
-					definition.ConcurrencyLimit,
-					definition.QueueCapacity,
-					definition.QueueCapacitySet,
-					tc.want.ConcurrencyLimit,
-					tc.want.QueueCapacity,
-					tc.wantSet,
-				)
-			}
-			policies := registry.CapacityPolicies()
-			got, ok := policies["custom"]
-			if ok != tc.wantSet || got != tc.want {
-				t.Fatalf("capacity policy=(%#v, %t), want (%#v, %t)", got, ok, tc.want, tc.wantSet)
-			}
-		})
-	}
-}
-
-func TestNewRegistryRejectsDuplicateIDs(t *testing.T) {
-	tests := []struct {
-		name      string
-		additions []domain.Backend
-		wantID    string
-	}{
-		{
-			name:      "OpenRouter collision after normalization",
-			additions: []domain.Backend{{ID: " openrouter "}},
-			wantID:    backend.OpenRouterID,
-		},
-		{
-			name:      "Rakestrawhome collision after normalization",
-			additions: []domain.Backend{{ID: " rakestrawhome "}},
-			wantID:    backend.RakestrawHomeID,
-		},
-		{
-			name: "consumer collision after normalization",
-			additions: []domain.Backend{
-				{ID: "custom", Endpoint: validEndpoint},
-				{ID: " custom ", Endpoint: "https://other.example/v1"},
-			},
-			wantID: "custom",
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			_, err := backend.NewRegistry(nil, tc.additions)
-			if err == nil {
-				t.Fatal("expected duplicate ID error")
-			}
-			if !strings.Contains(err.Error(), tc.wantID) {
-				t.Fatalf("expected error to identify %q, got %v", tc.wantID, err)
-			}
-		})
-	}
-}
-
-func TestNewRegistryValidatesIDs(t *testing.T) {
-	for _, id := range []string{"", " \t\n "} {
-		t.Run(id, func(t *testing.T) {
-			_, err := backend.NewRegistry(nil, []domain.Backend{{
-				ID:       id,
-				Endpoint: validEndpoint,
-			}})
-			if err == nil {
-				t.Fatal("expected blank ID error")
-			}
-		})
-	}
-}
-
-func TestNewRegistryValidatesEndpoints(t *testing.T) {
-	tests := []struct {
-		name     string
-		endpoint string
-	}{
-		{name: "blank", endpoint: ""},
-		{name: "relative", endpoint: "/v1"},
-		{name: "missing host", endpoint: "https:///v1"},
-		{name: "unsupported scheme", endpoint: "ftp://backend.example/v1"},
-		{name: "user information", endpoint: "https://user@backend.example/v1"},
-		{name: "query", endpoint: "https://backend.example/v1?mode=chat"},
-		{name: "empty query", endpoint: "https://backend.example/v1?"},
-		{name: "fragment", endpoint: "https://backend.example/v1#chat"},
-		{name: "empty fragment", endpoint: "https://backend.example/v1#"},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			_, err := backend.NewRegistry(nil, []domain.Backend{{
-				ID:       "custom",
-				Endpoint: tc.endpoint,
-			}})
-			if err == nil {
-				t.Fatal("expected invalid endpoint error")
-			}
-		})
-	}
-}
-
-func TestNewRegistryValidatesEnvironmentVariableNames(t *testing.T) {
-	for _, name := range []string{"1API_KEY", "API-KEY", "API KEY", "ÅPI_KEY"} {
-		t.Run(name, func(t *testing.T) {
-			_, err := backend.NewRegistry(nil, []domain.Backend{{
-				ID:        "custom",
-				Endpoint:  validEndpoint,
-				APIKeyEnv: name,
-			}})
-			if err == nil {
-				t.Fatal("expected invalid environment-variable name error")
-			}
-		})
-	}
-}
-
-func TestNewRegistryRejectsInvalidAndReservedExtraParameters(t *testing.T) {
-	tests := []struct {
-		name        string
-		extraParams map[string]any
-	}{
-		{name: "unsupported value", extraParams: map[string]any{"value": make(chan int)}},
-		{name: "reserved key", extraParams: map[string]any{"model": "override"}},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			_, err := backend.NewRegistry(nil, []domain.Backend{{
-				ID:          "custom",
-				Endpoint:    validEndpoint,
-				ExtraParams: tc.extraParams,
-			}})
-			if err == nil {
-				t.Fatal("expected invalid extra parameters error")
-			}
-		})
-	}
-}
-
-func TestRegistryLookupReportsNotFound(t *testing.T) {
-	registry, err := backend.NewRegistry(nil, nil)
+func TestEffectiveRegistryMissingBackend(t *testing.T) {
+	registry, err := backend.NewEffectiveRegistry(nil)
 	if err != nil {
-		t.Fatalf("construct registry: %v", err)
+		t.Fatal(err)
 	}
-
-	_, err = registry.GetBackend("missing")
-	if !errors.Is(err, backend.ErrBackendNotFound) {
-		t.Fatalf("expected ErrBackendNotFound, got %v", err)
+	if _, err := registry.GetBackend("missing"); !errors.Is(err, backend.ErrBackendNotFound) {
+		t.Fatalf("missing backend error = %v", err)
 	}
-	if !strings.Contains(err.Error(), "missing") {
-		t.Fatalf("expected error to identify backend, got %v", err)
+	if _, err := registry.GetBackend("local.v1"); !errors.Is(err, backend.ErrBackendNotFound) {
+		t.Fatalf("trimmed lookup unexpectedly matched: %v", err)
 	}
 }