Skip to content

fix(providers): fall back to configured models when upstream /models fails #266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SantiagoDePolonia merged 1 commit into from
Apr 24, 2026
Merged

fix(providers): fall back to configured models when upstream /models fails #266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 32 additions & 4 deletions internal/providers/azure/azure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package azure

import (
"context"
"log/slog"
"net/http"
"net/url"
"strconv"
Expand Down Expand Up @@ -30,17 +31,20 @@ type Provider struct {
resourceProvider *openai.CompatibleProvider
openAIResourceProvider *openai.CompatibleProvider
apiVersion string
configuredModels []string
}

func New(providerCfg providers.ProviderConfig, opts providers.ProviderOptions) core.Provider {
baseURL := providers.ResolveBaseURL(providerCfg.BaseURL, "https://example.invalid")
apiVersion := providers.ResolveAPIVersion(providerCfg.APIVersion, defaultAPIVersion)
p := &Provider{apiVersion: apiVersion}
clientCfg := openai.CompatibleProviderConfig{
ProviderName: "azure",
BaseURL: baseURL,
SetHeaders: setHeaders,
ProviderName: "azure",
BaseURL: baseURL,
SetHeaders: setHeaders,
ConfiguredModels: opts.Models,
}
p.configuredModels = opts.Models
p.CompatibleProvider = openai.NewCompatibleProvider(providerCfg.APIKey, opts, clientCfg)
p.resourceProvider = openai.NewCompatibleProvider(providerCfg.APIKey, opts, clientCfg)
p.openAIResourceProvider = openai.NewCompatibleProvider(providerCfg.APIKey, opts, clientCfg)
Expand Down Expand Up @@ -80,7 +84,31 @@ func (p *Provider) ListModels(ctx context.Context) (*core.ModelsResponse, error)
Method: http.MethodGet,
Endpoint: "/openai/models",
}, &resp); err != nil {
return nil, err
if len(p.configuredModels) == 0 {
return nil, err
}

slog.Warn("azure upstream ListModels failed, using configured models fallback",
"error", err,
"configured_models", len(p.configuredModels),
)

data := make([]core.Model, 0, len(p.configuredModels))
for _, modelID := range p.configuredModels {
modelID = strings.TrimSpace(modelID)
if modelID == "" {
continue
}
data = append(data, core.Model{
ID: modelID,
Object: "model",
OwnedBy: "azure",
})
}
return &core.ModelsResponse{
Object: "list",
Data: data,
}, nil
Comment on lines +96 to +111

@greptile-apps greptile-apps Bot Apr 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Azure fallback skips deduplication

The inline fallback loop in Azure's ListModels trims and skips empty entries, but does not deduplicate. If a user's configuration contains duplicate model IDs (e.g. ["my-model", "my-model"]), the response will contain duplicate entries. The normalizeConfiguredModels helper introduced in this PR handles exactly this — it trims, filters blanks, and deduplicates. For consistency, consider pre-normalizing the models in New before assigning to p.configuredModels rather than repeating the logic inline.

}
return &resp, nil
}
Expand Down
106 changes: 92 additions & 14 deletions internal/providers/openai/compatible_provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ package openai
import (
"context"
"io"
"log/slog"
"net/http"
"net/url"
"strconv"
"strings"

"gomodel/internal/core"
"gomodel/internal/llmclient"
Expand All @@ -15,24 +17,27 @@ import (
type RequestMutator func(*llmclient.Request)

type CompatibleProviderConfig struct {
ProviderName string
BaseURL string
SetHeaders func(*http.Request, string)
RequestMutator RequestMutator
ProviderName string
BaseURL string
SetHeaders func(*http.Request, string)
RequestMutator RequestMutator
ConfiguredModels []string
}

type CompatibleProvider struct {
client *llmclient.Client
apiKey string
providerName string
requestMutator RequestMutator
client *llmclient.Client
apiKey string
providerName string
requestMutator RequestMutator
configuredModels []string
}

func NewCompatibleProvider(apiKey string, opts providers.ProviderOptions, cfg CompatibleProviderConfig) *CompatibleProvider {
p := &CompatibleProvider{
apiKey: apiKey,
providerName: cfg.ProviderName,
requestMutator: cfg.RequestMutator,
apiKey: apiKey,
providerName: cfg.ProviderName,
requestMutator: cfg.RequestMutator,
configuredModels: normalizeConfiguredModels(cfg.ConfiguredModels),
}
clientCfg := llmclient.Config{
ProviderName: cfg.ProviderName,
Expand All @@ -54,9 +59,10 @@ func NewCompatibleProviderWithHTTPClient(apiKey string, httpClient *http.Client,
httpClient = http.DefaultClient
}
p := &CompatibleProvider{
apiKey: apiKey,
providerName: cfg.ProviderName,
requestMutator: cfg.RequestMutator,
apiKey: apiKey,
providerName: cfg.ProviderName,
requestMutator: cfg.RequestMutator,
configuredModels: normalizeConfiguredModels(cfg.ConfiguredModels),
}
clientCfg := llmclient.DefaultConfig(cfg.ProviderName, cfg.BaseURL)
clientCfg.Hooks = hooks
Expand Down Expand Up @@ -127,6 +133,53 @@ func (p *CompatibleProvider) StreamChatCompletion(ctx context.Context, req *core
}

func (p *CompatibleProvider) ListModels(ctx context.Context) (*core.ModelsResponse, error) {
if len(p.configuredModels) == 0 {
return p.doListModels(ctx)
}

resp, err := p.doListModels(ctx)
if err != nil {
slog.Warn("openai-compatible upstream ListModels failed, using configured models fallback",
"provider", p.providerName,
"error", err,
"configured_models", len(p.configuredModels),
)
}

byID := make(map[string]core.Model, len(p.configuredModels))
if resp != nil {
for _, model := range resp.Data {
byID[strings.TrimSpace(model.ID)] = model
}
}

data := make([]core.Model, 0, len(p.configuredModels))
for _, modelID := range p.configuredModels {
model, ok := byID[modelID]
if !ok {
model = core.Model{
ID: modelID,
Object: "model",
OwnedBy: p.providerName,
}
} else {
if strings.TrimSpace(model.Object) == "" {
model.Object = "model"
}
if strings.TrimSpace(model.OwnedBy) == "" {
model.OwnedBy = p.providerName
}
}
data = append(data, model)
}

return &core.ModelsResponse{
Object: "list",
Data: data,
}, nil
Comment on lines 135 to +179

@greptile-apps greptile-apps Bot Apr 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Configured models now act as an allow-list, not just a fallback

When configuredModels is non-empty and the upstream /models endpoint succeeds, the new logic still returns only the configured models (enriched with upstream metadata). Upstream models that are NOT in the configured list are silently dropped.

This is a subtle semantic shift for providers like openai, openrouter, and vllm where /models works correctly: a user who has specified e.g. models: ["gpt-4o"] to set a preferred default will now see only gpt-4o from ListModels, losing visibility into every other available model. Previously the models config had no effect on what ListModels returned for these providers.

If the intent is purely "fallback when upstream is unavailable," the success path could instead return the upstream response directly (unchanged), and only use the configured list when err != nil. That would preserve the old behavior for working endpoints while still adding the desired resilience:

func (p *CompatibleProvider) ListModels(ctx context.Context) (*core.ModelsResponse, error) {
    resp, err := p.doListModels(ctx)
    if err == nil {
        return resp, nil // upstream worked — return as-is
    }
    if len(p.configuredModels) == 0 {
        return nil, err
    }
    // fallback to configured models ...
}

If the allow-list behaviour is intentional (i.e. models: is meant to restrict what users can see), that's fine — but it's worth confirming, since it changes the existing contract for OpenAI/OpenRouter/vllm.

}
Comment on lines 135 to +180

@coderabbitai coderabbitai Bot Apr 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

On upstream success, configured models silently filter out any upstream model not listed in config.

When configuredModels is non-empty and doListModels succeeds, the returned Data is built by iterating only p.configuredModels. Any upstream model not present in the configured list is dropped. This is a significant behavior change: before this PR, a provider configured with e.g. models: [gpt-4o] and successful upstream discovery returned the full /models list; now it returns only gpt-4o. Users set models: for many reasons (hinting, routing, cost) — not necessarily to scope discovery — and this turns that field into a whitelist retroactively.

The PR description frames ConfiguredModels as a reliability fallback ("preserving upstream metadata when available"), so keeping upstream-only entries on success would match the stated intent. If filtering is intentional, it should be an opt-in and explicitly documented on CompatibleProviderConfig.ConfiguredModels.

Proposed fix: keep upstream-only models when the upstream call succeeded 
 func (p *CompatibleProvider) ListModels(ctx context.Context) (*core.ModelsResponse, error) {
 	if len(p.configuredModels) == 0 {
 		return p.doListModels(ctx)
 	}
 
 	resp, err := p.doListModels(ctx)
 	if err != nil {
 		slog.Warn("openai-compatible upstream ListModels failed, using configured models fallback",
 			"provider", p.providerName,
 			"error", err,
 			"configured_models", len(p.configuredModels),
 		)
 	}
 
 	byID := make(map[string]core.Model, len(p.configuredModels))
 	if resp != nil {
 		for _, model := range resp.Data {
 			byID[strings.TrimSpace(model.ID)] = model
 		}
 	}
 
-	data := make([]core.Model, 0, len(p.configuredModels))
-	for _, modelID := range p.configuredModels {
-		model, ok := byID[modelID]
-		if !ok {
-			model = core.Model{
-				ID:      modelID,
-				Object:  "model",
-				OwnedBy: p.providerName,
-			}
-		} else {
-			if strings.TrimSpace(model.Object) == "" {
-				model.Object = "model"
-			}
-			if strings.TrimSpace(model.OwnedBy) == "" {
-				model.OwnedBy = p.providerName
-			}
-		}
-		data = append(data, model)
-	}
+	seen := make(map[string]struct{}, len(p.configuredModels))
+	data := make([]core.Model, 0, len(p.configuredModels))
+	for _, modelID := range p.configuredModels {
+		model, ok := byID[modelID]
+		if !ok {
+			model = core.Model{ID: modelID, Object: "model", OwnedBy: p.providerName}
+		} else {
+			if strings.TrimSpace(model.Object) == "" {
+				model.Object = "model"
+			}
+			if strings.TrimSpace(model.OwnedBy) == "" {
+				model.OwnedBy = p.providerName
+			}
+		}
+		seen[modelID] = struct{}{}
+		data = append(data, model)
+	}
+	// Preserve upstream-only entries when upstream discovery succeeded.
+	if err == nil && resp != nil {
+		for _, model := range resp.Data {
+			if _, dup := seen[strings.TrimSpace(model.ID)]; dup {
+				continue
+			}
+			data = append(data, model)
+		}
+	}
 
 	return &core.ModelsResponse{
 		Object: "list",
 		Data:   data,
 	}, nil
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@internal/providers/openai/compatible_provider.go` around lines 135 - 180, The
current ListModels (CompatibleProvider.ListModels) drops upstream-only models
when p.configuredModels is non-empty even if doListModels succeeded; change the
logic so when doListModels returns successfully (err == nil and resp != nil) you
merge upstream resp.Data with configuredModels instead of iterating only
configuredModels: build byID from resp.Data, then for each configured modelID
ensure an entry exists or is updated (set ID, Object="model" if empty,
OwnedBy=p.providerName if empty) so configured entries can augment/override
upstream metadata, and finally produce ModelsResponse.Data as the union of all
upstream models plus any configured-only models; preserve the current fallback
behavior (use only configuredModels) when doListModels failed (err != nil).


func (p *CompatibleProvider) doListModels(ctx context.Context) (*core.ModelsResponse, error) {
var resp core.ModelsResponse
err := p.Do(ctx, llmclient.Request{
Method: http.MethodGet,
Expand Down Expand Up @@ -492,3 +545,28 @@ func responseInputItemsEndpoint(id string, params core.ResponseInputItemsParams)
}
return endpoint
}

// normalizeConfiguredModels deduplicates and trims model names.
func normalizeConfiguredModels(models []string) []string {
if len(models) == 0 {
return nil
}

seen := make(map[string]struct{}, len(models))
normalized := make([]string, 0, len(models))
for _, model := range models {
model = strings.TrimSpace(model)
if model == "" {
continue
}
if _, exists := seen[model]; exists {
continue
}
seen[model] = struct{}{}
normalized = append(normalized, model)
}
if len(normalized) == 0 {
return nil
}
return normalized
}
Comment on lines +549 to +572

@coderabbitai coderabbitai Bot Apr 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Consider case-insensitive de-duplication, or document the case-sensitive contract.

normalizeConfiguredModels de-dupes by exact string after trimming. Most providers treat model IDs case-insensitively (the o-series and gpt-5 detectors in openai.go lowercase before comparing, Line 87 and Line 94), so a config with ["GPT-4o", "gpt-4o"] will produce two entries here and two rows in the fallback. Either lowercase the key used for seen (keeping the original casing in normalized) or document that IDs are treated as case-sensitive.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@internal/providers/openai/compatible_provider.go` around lines 549 - 572,
normalizeConfiguredModels currently deduplicates model names case-sensitively
which can produce duplicates like "GPT-4o" and "gpt-4o"; update
normalizeConfiguredModels to use a case-insensitive key for the seen map (e.g.
use strings.ToLower(model) when checking/setting seen) while still appending the
original trimmed model to the normalized slice so external casing is preserved;
reference the normalizeConfiguredModels function to locate the change and ensure
behavior aligns with the lowercase checks used elsewhere (e.g., the
o-series/gpt-5 detectors in openai.go).

Loading