fix: handle keyless providers in initial list models requests - #833
Conversation
🧪 Test Suite AvailableThis PR can be tested by a repository admin. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughList models flows updated to support key-less custom providers: bifrost skips key retrieval when providerRequiresKey is false; getProviderByKey now loads provider config from account, uses a per-provider mutex, double-checks providers, and prepares the provider under the lock. Multiple providers route key-less ListModels to single-key handlers. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Bifrost as Bifrost.ListModels
participant Account as Account.GetConfigForProvider
participant ProviderReg as providers registry
participant Provider as Provider.ListModels
participant KeyHandler as Provider.listModelsByKey
Client->>Bifrost: ListModelsRequest
Bifrost->>Bifrost: providerRequiresKey(provider, cfg)?
alt providerRequiresKey == false (key-less)
Bifrost->>Bifrost: keys = nil (skip getAllSupportedKeys)
else providerRequiresKey == true
Bifrost->>Bifrost: getAllSupportedKeys(...)
end
Bifrost->>ProviderReg: getProviderByKey(providerName)
ProviderReg->>ProviderReg: provider found?
alt not found
ProviderReg->>Account: GetConfigForProvider(providerName)
Account-->>ProviderReg: providerConfig
ProviderReg->>ProviderReg: lock per-provider mutex
ProviderReg->>ProviderReg: double-check providers slice
ProviderReg->>ProviderReg: prepareProvider(providerConfig)
ProviderReg-->>Bifrost: prepared provider
else found
ProviderReg-->>Bifrost: provider
end
Bifrost->>Provider: ListModels(request, keys)
alt customProviderConfig && IsKeyLess
Provider->>KeyHandler: listModelsByKey(empty_key, request)
KeyHandler-->>Provider: models response
else
Provider->>Provider: standard multi-key aggregation
end
Provider-->>Client: BifrostListModelsResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧬 Code graph analysis (3)core/bifrost.go (2)
core/providers/gemini/gemini.go (1)
core/providers/anthropic/anthropic.go (1)
🔇 Additional comments (4)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
core/providers/cohere/cohere.go (1)
260-262: Keyless ListModels path looks good; use request.PageSize when provided.The guard/early-return is correct. In listModelsByKey we still send DefaultPageSize—prefer request.PageSize (bounded) when set to reduce work and align with client intent.
core/providers/gemini/gemini.go (1)
182-184: Keyless routing correct; implement pagination parameter handling.Early-return to listModelsByKey for keyless configs is correct and consistent. However, verification confirms the pagination concern: listModelsByKey accepts a request with PageSize and PageToken fields but ignores both, hardcoding DefaultPageSize in the URL (line 139) and not passing pageToken. The Gemini API supports both parameters—update line 139 to use
request.PageSizeif provided (fallback to DefaultPageSize), and appendrequest.PageTokento the URL when present to support cursor-based pagination and reduce over-fetching.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
core/bifrost.go(2 hunks)core/changelog.md(1 hunks)core/providers/anthropic/anthropic.go(1 hunks)core/providers/cohere/cohere.go(1 hunks)core/providers/gemini/gemini.go(1 hunks)core/providers/openai/openai.go(1 hunks)transports/changelog.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
core/providers/cohere/cohere.go (1)
core/schemas/account.go (1)
Key(8-17)
core/bifrost.go (2)
core/schemas/account.go (1)
Key(8-17)core/schemas/provider.go (2)
CustomProviderConfig(142-148)Provider(207-234)
core/providers/gemini/gemini.go (1)
core/schemas/account.go (1)
Key(8-17)
core/providers/anthropic/anthropic.go (1)
core/schemas/account.go (1)
Key(8-17)
core/providers/openai/openai.go (3)
core/schemas/bifrost.go (1)
ListModelsRequest(84-84)core/schemas/account.go (1)
Key(8-17)core/providers/utils/utils.go (1)
ShouldSendBackRawResponse(480-485)
🔇 Additional comments (4)
core/changelog.md (1)
2-3: Changelog entries read well.No issues from my side.
core/providers/openai/openai.go (1)
86-96: Keyless ListModels routing is correct.Early-return to listModelsByKeyOpenAI for IsKeyLess is clean and leverages existing URL/path override logic. LGTM.
transports/changelog.md (1)
4-5: Changelog updates match the code changes.Looks good.
core/bifrost.go (1)
278-284: Verification confirmed: guard is correctly implemented.The
providerRequiresKeyfunction at core/utils.go:54-59 properly handles theIsKeyLessflag. It returns false whencustomConfig.IsKeyLessis true (except intentionally for Bedrock per the "not allowed" comment), and also handles native keyless providers (Ollama, SGL). All custom provider implementations (OpenAI, Cohere, Gemini, Anthropic) respect this flag, confirming the keyless flow is properly supported across intended providers.
7eeb4f6 to
165b8e6
Compare
Merge activity
|
## Summary Fixed issues with custom keyless providers during initial list models requests, ensuring proper initialization and handling of providers that don't require API keys. ## Changes - Added a check to skip key retrieval for providers that don't require keys - Implemented auto-initialization of providers when they're not yet initialized but have a valid config - Added support for keyless model listing in Anthropic, Cohere, Gemini, and OpenAI providers - Updated provider implementations to handle keyless configurations properly ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Test listing models with custom keyless providers: ```sh # Core/Transports go version go test ./... # Test with a custom keyless provider configuration curl -X GET "http://localhost:8000/v1/models" -H "Authorization: Bearer your_token" ``` ## Breaking changes - [ ] Yes - [x] No ## Related issues Fixes issues with custom keyless providers failing to initialize or list models properly. ## Security considerations This change maintains the security model by properly handling keyless provider configurations without exposing sensitive information. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
1 participant
Summary
Fixed issues with custom keyless providers during initial list models requests, ensuring proper initialization and handling of providers that don't require API keys.
Changes
Type of change
Affected areas
How to test
Test listing models with custom keyless providers:
Breaking changes
Related issues
Fixes issues with custom keyless providers failing to initialize or list models properly.
Security considerations
This change maintains the security model by properly handling keyless provider configurations without exposing sensitive information.
Checklist
docs/contributing/README.mdand followed the guidelines