From 0988140369e4326fe77bd07b4205405fc87f6fec Mon Sep 17 00:00:00 2001 From: "Jakub A. W" Date: Tue, 12 May 2026 17:03:06 +0200 Subject: [PATCH] refactor(router): inline 3 core resolver methods into ModelLookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Router.go ran three runtime type-assertions on `r.lookup` against the optional interfaces `core.ProviderNameResolver`, `core.ProviderTypeNameResolver`, and `core.ProviderNameTypeResolver`. *ModelRegistry already implements all three; the assertions existed because the test `mockModelLookup` deliberately omitted them to keep its surface small. The optional-protocol pattern was being used as feature detection rather than as real interface segregation: every production call site assumed the assertion would succeed. Widening `core.ModelLookup` with the three methods removes the indirection — `r.lookup.GetProviderName(...)` instead of `if named, ok := r.lookup.(core.ProviderNameResolver); ok { named.GetProviderName(...) }` — and the mock gains three trivial stub methods returning empty string, matching how its missing data was already surfaced. The three `core.*Resolver` interface types stay defined; they are still used in 5 other places for provider-side type assertions on `core.Provider` (in internal/server, internal/aliases, internal/gateway). Only the lookup-side assertions are removed. One side-effect cleanup in `Router.GetProviderTypeForName`: the `modelWithProviderLister` iteration fallback became unreachable now that the direct method is always available, and is removed. Behavior is strictly more complete than the fallback — *ModelRegistry's direct method consults the provider name/type maps, which include registered providers with zero discovered models that the model-iteration fallback would have missed. Test plan: - All 25-ish tests using mockModelLookup compile and pass under -race. - Full `go test ./...` clean across 56 packages. - `*ModelRegistry` satisfies the widened interface (compile-time assertion at registry_test.go:1865 still holds). Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/core/interfaces.go | 15 +++++++++++++++ internal/providers/router.go | 32 ++++--------------------------- internal/providers/router_test.go | 7 +++++++ 3 files changed, 26 insertions(+), 28 deletions(-) diff --git a/internal/core/interfaces.go b/internal/core/interfaces.go index 3dfc05b8..eb87f635 100644 --- a/internal/core/interfaces.go +++ b/internal/core/interfaces.go @@ -194,4 +194,19 @@ type ModelLookup interface { // ModelCount returns the number of registered models ModelCount() int + + // GetProviderName maps a model selector back to the concrete configured + // provider instance name. Implementations that have no such mapping return + // an empty string. Same shape as the optional ProviderNameResolver + // interface used elsewhere for provider-side type assertions. + GetProviderName(model string) string + + // GetProviderNameForType maps a provider type such as "openai" to the + // concrete configured instance name chosen for routing, e.g. + // "openai-primary". Returns empty when no mapping exists. + GetProviderNameForType(providerType string) string + + // GetProviderTypeForName maps a concrete configured instance name back to + // its provider type. Returns empty when no mapping exists. + GetProviderTypeForName(providerName string) string } diff --git a/internal/providers/router.go b/internal/providers/router.go index 4ea165f3..d5df805b 100644 --- a/internal/providers/router.go +++ b/internal/providers/router.go @@ -112,12 +112,7 @@ func (r *Router) resolveUnqualifiedSelector(selector core.ModelSelector) (core.M if selector.Provider != "" || strings.TrimSpace(selector.Model) == "" { return core.ModelSelector{}, false } - - named, ok := r.lookup.(core.ProviderNameResolver) - if !ok { - return core.ModelSelector{}, false - } - providerName := strings.TrimSpace(named.GetProviderName(selector.Model)) + providerName := strings.TrimSpace(r.lookup.GetProviderName(selector.Model)) if providerName == "" { return core.ModelSelector{}, false } @@ -613,19 +608,13 @@ func (r *Router) GetProviderName(model string) string { if selector.Provider != "" { return selector.Provider } - if named, ok := r.lookup.(core.ProviderNameResolver); ok { - return named.GetProviderName(selector.QualifiedModel()) - } - return "" + return r.lookup.GetProviderName(selector.QualifiedModel()) } // GetProviderNameForType returns the concrete configured provider instance name // chosen for a provider-typed route. func (r *Router) GetProviderNameForType(providerType string) string { - if named, ok := r.lookup.(core.ProviderTypeNameResolver); ok { - return strings.TrimSpace(named.GetProviderNameForType(providerType)) - } - return "" + return strings.TrimSpace(r.lookup.GetProviderNameForType(providerType)) } // GetProviderTypeForName returns the provider type for a concrete configured @@ -635,20 +624,7 @@ func (r *Router) GetProviderTypeForName(providerName string) string { if providerName == "" { return "" } - if typed, ok := r.lookup.(core.ProviderNameTypeResolver); ok { - return strings.TrimSpace(typed.GetProviderTypeForName(providerName)) - } - if models, ok := r.lookup.(modelWithProviderLister); ok { - for _, entry := range models.ListModelsWithProvider() { - if strings.TrimSpace(entry.ProviderName) != providerName { - continue - } - if providerType := strings.TrimSpace(entry.ProviderType); providerType != "" { - return providerType - } - } - } - return "" + return strings.TrimSpace(r.lookup.GetProviderTypeForName(providerName)) } func (r *Router) providerByType(providerType string) core.Provider { diff --git a/internal/providers/router_test.go b/internal/providers/router_test.go index efad078c..fc8b8523 100644 --- a/internal/providers/router_test.go +++ b/internal/providers/router_test.go @@ -104,6 +104,13 @@ func (m *mockModelLookup) ModelCount() int { return len(m.models) } +// The mock keeps no provider-name <-> type mapping, so the three resolver +// methods always return empty. Tests that need provider-name routing use the +// real ModelRegistry via newTestRegistryWithModels instead of this mock. +func (m *mockModelLookup) GetProviderName(_ string) string { return "" } +func (m *mockModelLookup) GetProviderNameForType(_ string) string { return "" } +func (m *mockModelLookup) GetProviderTypeForName(_ string) string { return "" } + // mockProvider is a simple mock implementation of core.Provider for testing type mockProvider struct { name string