feat: scope ListModels to single key when KeyID is set, add filterKeysByID helper - #4193
Conversation
|
Warning Review limit reached
More reviews will be available in 36 minutes and 41 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
|
Confidence Score: 5/5Safe to merge; the change is narrow, pool-safe, and the key filtering logic is straightforward. The feature is a clean, well-tested addition. The only concern is that the "no key found" error leaves fallback behavior open, which could silently route to a different provider when fallbacks are configured — a minor edge case unlikely to occur in practice for ListModels requests. core/bifrost.go — specifically the AllowFallbacks omission in the new error path at the KeyID filter block. Important Files Changed
Reviews (4): Last reviewed commit: "feat: adds key param in list models" | Re-trigger Greptile |
0947354 to
e042428
Compare
29ed4c0 to
267d94f
Compare
e042428 to
89bb042
Compare
267d94f to
5952bf3
Compare
Merge activity
|
5952bf3 to
499ef15
Compare
…erKeysByID` helper (#4193) ## Summary Adds a `KeyID` field to `BifrostListModelsRequest` that scopes a `ListModels` call to a single key matched by `Key.ID`. This allows callers such as the catalog composer to cache list-models output per-key for fine-grained invalidation without requiring an extra round-trip or having the provider aggregate results across every configured key. ## Changes - Added `KeyID *string` to `BifrostListModelsRequest` (tagged `json:"-"` so it is never forwarded to providers). When set, the request worker filters the already-validated key set down to the single matching key before dispatching. If no key matches, a `BifrostError` is returned immediately. - Added `filterKeysByID` helper that returns a new slice containing only keys whose `ID` equals the target, leaving the input slice unmodified. - Added `TestFilterKeysByID` covering: a successful match, a missing key, an empty target string, and input-slice immutability. Note: the lookup runs against the already-filtered set of supported keys (disabled or invalid keys are excluded before the match), so a `KeyID` pointing to a disabled key produces the same "no key found" error as a non-existent `KeyID`. Callers that need to distinguish these cases must inspect the raw account configuration directly. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go version go test ./... ``` The new `TestFilterKeysByID` test directly exercises the helper and the key-scoping logic. To validate end-to-end, issue a `ListModels` request with `KeyID` set to a valid key ID and confirm only that key's models are returned. Issue one with an unknown `KeyID` and confirm a "no key found" error is returned. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations `KeyID` is tagged `json:"-"` and is never serialised or forwarded to any external provider. No secrets or PII are introduced. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…erKeysByID` helper (maximhq#4193) ## Summary Adds a `KeyID` field to `BifrostListModelsRequest` that scopes a `ListModels` call to a single key matched by `Key.ID`. This allows callers such as the catalog composer to cache list-models output per-key for fine-grained invalidation without requiring an extra round-trip or having the provider aggregate results across every configured key. ## Changes - Added `KeyID *string` to `BifrostListModelsRequest` (tagged `json:"-"` so it is never forwarded to providers). When set, the request worker filters the already-validated key set down to the single matching key before dispatching. If no key matches, a `BifrostError` is returned immediately. - Added `filterKeysByID` helper that returns a new slice containing only keys whose `ID` equals the target, leaving the input slice unmodified. - Added `TestFilterKeysByID` covering: a successful match, a missing key, an empty target string, and input-slice immutability. Note: the lookup runs against the already-filtered set of supported keys (disabled or invalid keys are excluded before the match), so a `KeyID` pointing to a disabled key produces the same "no key found" error as a non-existent `KeyID`. Callers that need to distinguish these cases must inspect the raw account configuration directly. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go version go test ./... ``` The new `TestFilterKeysByID` test directly exercises the helper and the key-scoping logic. To validate end-to-end, issue a `ListModels` request with `KeyID` set to a valid key ID and confirm only that key's models are returned. Issue one with an unknown `KeyID` and confirm a "no key found" error is returned. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations `KeyID` is tagged `json:"-"` and is never serialised or forwarded to any external provider. No secrets or PII are introduced. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…erKeysByID` helper (maximhq#4193) ## Summary Adds a `KeyID` field to `BifrostListModelsRequest` that scopes a `ListModels` call to a single key matched by `Key.ID`. This allows callers such as the catalog composer to cache list-models output per-key for fine-grained invalidation without requiring an extra round-trip or having the provider aggregate results across every configured key. ## Changes - Added `KeyID *string` to `BifrostListModelsRequest` (tagged `json:"-"` so it is never forwarded to providers). When set, the request worker filters the already-validated key set down to the single matching key before dispatching. If no key matches, a `BifrostError` is returned immediately. - Added `filterKeysByID` helper that returns a new slice containing only keys whose `ID` equals the target, leaving the input slice unmodified. - Added `TestFilterKeysByID` covering: a successful match, a missing key, an empty target string, and input-slice immutability. Note: the lookup runs against the already-filtered set of supported keys (disabled or invalid keys are excluded before the match), so a `KeyID` pointing to a disabled key produces the same "no key found" error as a non-existent `KeyID`. Callers that need to distinguish these cases must inspect the raw account configuration directly. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go version go test ./... ``` The new `TestFilterKeysByID` test directly exercises the helper and the key-scoping logic. To validate end-to-end, issue a `ListModels` request with `KeyID` set to a valid key ID and confirm only that key's models are returned. Issue one with an unknown `KeyID` and confirm a "no key found" error is returned. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations `KeyID` is tagged `json:"-"` and is never serialised or forwarded to any external provider. No secrets or PII are introduced. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable

Summary
Adds a
KeyIDfield toBifrostListModelsRequestthat scopes aListModelscall to a single key matched byKey.ID. This allows callers such as the catalog composer to cache list-models output per-key for fine-grained invalidation without requiring an extra round-trip or having the provider aggregate results across every configured key.Changes
KeyID *stringtoBifrostListModelsRequest(taggedjson:"-"so it is never forwarded to providers). When set, the request worker filters the already-validated key set down to the single matching key before dispatching. If no key matches, aBifrostErroris returned immediately.filterKeysByIDhelper that returns a new slice containing only keys whoseIDequals the target, leaving the input slice unmodified.TestFilterKeysByIDcovering: a successful match, a missing key, an empty target string, and input-slice immutability.Note: the lookup runs against the already-filtered set of supported keys (disabled or invalid keys are excluded before the match), so a
KeyIDpointing to a disabled key produces the same "no key found" error as a non-existentKeyID. Callers that need to distinguish these cases must inspect the raw account configuration directly.Type of change
Affected areas
How to test
go version go test ./...The new
TestFilterKeysByIDtest directly exercises the helper and the key-scoping logic. To validate end-to-end, issue aListModelsrequest withKeyIDset to a valid key ID and confirm only that key's models are returned. Issue one with an unknownKeyIDand confirm a "no key found" error is returned.Screenshots/Recordings
N/A
Breaking changes
Related issues
Security considerations
KeyIDis taggedjson:"-"and is never serialised or forwarded to any external provider. No secrets or PII are introduced.Checklist
docs/contributing/README.mdand followed the guidelines