fix: return informative message when model list is disabled for provider - #5895
gokul-scalent wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesProvider model listing
Estimated code review effort: 2 (Simple) | ~15 minutes Mergeability Score: ⚪ Minimal · up to The change excludes providers with model listing disabled and returns an informative response for explicit provider requests; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/bifrost.go`:
- Around line 3605-3627: Update the model-provider collection around
GetConfiguredProviders and the modelProviders append to include only providers
that successfully completed initialization, reusing the existing
initialized-provider availability state or predicate. Preserve the current
config and operation-allowance filtering, and add a regression test covering a
prepareProvider failure so ListAllModels excludes that provider.
In `@transports/bifrost-http/handlers/inference_list_models_test.go`:
- Around line 100-105: Update TestBuildDisabledListModelsResponse to explicitly
assert that resp.Data is non-nil before asserting it is empty, ensuring the
response serializes data as an empty slice rather than null.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 533cb662-8817-480f-8e3f-65b19a281612
📒 Files selected for processing (5)
core/bifrost.gocore/bifrost_test.gocore/schemas/models.gotransports/bifrost-http/handlers/inference.gotransports/bifrost-http/handlers/inference_list_models_test.go
244a01d to
ce1b2a6
Compare
b90c437 to
5409536
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
New PR created for this |
fix: respect list_models disabled setting in /v1/models fan-out
Summary
When a provider had
list_modelsdisabled viaAllowedRequests, theGET /v1/modelsendpoint would still fan out to that provider and include its models in the response (or return an error). Two separate code paths were affected:GetConfiguredProviders()returned all providers regardless of theirAllowedRequestsconfig, causing disabled providers to be included in the aggregate model list.GET /v1/models?provider=<name>did not checkAllowedRequestsbefore making the request, resulting in anunsupported_operationerror being surfaced to the caller.Changes
GetConfiguredProviders()now filters out providers wherelist_modelsis explicitly disabled in theirCustomProviderConfig.AllowedRequests. Providers with noAllowedRequestsconfig (i.e. all operations allowed) are unaffected.messagefield toBifrostListModelsResponseto convey informational messages (e.g. disabled notice) without breaking the response shape.listModels: if a specific provider is requested and itslist_modelsis disabled, return an empty model list with a human-readable message ("The model_list request is disabled for this provider.") instead of forwarding the request.inference_list_models_test.go— Added unit tests covering both fan-out filtering and single-provider disabled behavior.Type of change
Affected areas
How to test
Describe the steps to validate this change. Include commands and expected outcomes.


Screenshots/Recordings
Previously, requests to providers with model_list disabled resulted in error logs. This change ensures those providers are excluded from model_list fan-out, eliminating the unnecessary errors.
Breaking changes
If yes, describe impact and migration instructions.
Related issues
Link related issues and discussions. Example: Closes #123
Security considerations
Note any security implications (auth, secrets, PII, sandboxing, etc.).
Checklist
docs/contributing/README.mdand followed the guidelines