fix: add mantle list models endpoint in bedrock list models method - #4677
Conversation
|
tejas ghatte seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
📝 WalkthroughWalkthroughThe Bedrock provider's ChangesBedrock Mantle Model Listing
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/providers/bedrock/bedrock.go (1)
865-881: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMantle fetch now runs on every
listModelsByKeycall.
listMantleModelsissues an extra signed GET (via the unaryprovider.client) on every list-models request. It's best-effort (nil on failure), but an unreachable/slow Mantle endpoint can add up to the unary client's timeout to each call, andstartTime(L791) now folds the Mantle round-trip into the reportedLatency. Consider whether this should be gated (e.g., only when Mantle is configured/relevant) and confirm the latency semantics are acceptable.🤖 Prompt for 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. In `@core/providers/bedrock/bedrock.go` around lines 865 - 881, The Mantle merge in listModelsByKey currently triggers listMantleModels on every request, which adds an extra signed client call and inflates the reported Latency. Update the logic around listModelsByKey and listMantleModels to only fetch Mantle models when Mantle is actually configured/relevant (or another explicit gate applies), and make sure the latency measurement in response.ExtraFields.Latency reflects the intended scope of the call.
🤖 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.
Nitpick comments:
In `@core/providers/bedrock/bedrock.go`:
- Around line 865-881: The Mantle merge in listModelsByKey currently triggers
listMantleModels on every request, which adds an extra signed client call and
inflates the reported Latency. Update the logic around listModelsByKey and
listMantleModels to only fetch Mantle models when Mantle is actually
configured/relevant (or another explicit gate applies), and make sure the
latency measurement in response.ExtraFields.Latency reflects the intended scope
of the call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 47ffe943-ffbb-40a7-806a-d00433c18912
📒 Files selected for processing (1)
core/providers/bedrock/bedrock.go
Merge activity
|
…aximhq#4677) ## Summary Bedrock's `ListFoundationModels` API omits models served exclusively through the Mantle OpenAI-compatible endpoint (e.g. `gpt-5.x`, `gemma-4`). This PR adds a supplementary call to the Mantle `/v1/models` endpoint and merges the results into the existing foundation model list, ensuring those models are discoverable through Bifrost's list models API. ## Changes - Added `listMantleModels` which calls the Bedrock Mantle `/v1/models` endpoint, supporting both Bearer token and AWS SigV4 signing. The function is best-effort and returns `nil` on any failure so the foundation model list is always returned. - After the standard `listModelsByKey` response is built, the Mantle catalog is merged in, deduplicating by model ID and applying the same allow/blacklist/alias gating as the foundation model path. - Reused the existing `OpenAIListModelsResponse.ToBifrostListModelsResponse` conversion to normalize Mantle model entries into the Bifrost schema. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Call the Bifrost list models endpoint against a Bedrock key configured for a region where Mantle models are available and verify that models such as `gpt-5.x` or `gemma-4` appear in the response alongside the standard foundation models. ```sh go test ./... ``` Confirm that a failure to reach the Mantle endpoint (e.g. network error, non-200 response) does not prevent the foundation model list from being returned. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations The Mantle request uses the same signing path (`signAWSRequestFromKey`) as other Bedrock requests, or falls back to a Bearer token if a key value is present. No new credentials or secrets are introduced. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…aximhq#4677) ## Summary Bedrock's `ListFoundationModels` API omits models served exclusively through the Mantle OpenAI-compatible endpoint (e.g. `gpt-5.x`, `gemma-4`). This PR adds a supplementary call to the Mantle `/v1/models` endpoint and merges the results into the existing foundation model list, ensuring those models are discoverable through Bifrost's list models API. ## Changes - Added `listMantleModels` which calls the Bedrock Mantle `/v1/models` endpoint, supporting both Bearer token and AWS SigV4 signing. The function is best-effort and returns `nil` on any failure so the foundation model list is always returned. - After the standard `listModelsByKey` response is built, the Mantle catalog is merged in, deduplicating by model ID and applying the same allow/blacklist/alias gating as the foundation model path. - Reused the existing `OpenAIListModelsResponse.ToBifrostListModelsResponse` conversion to normalize Mantle model entries into the Bifrost schema. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Call the Bifrost list models endpoint against a Bedrock key configured for a region where Mantle models are available and verify that models such as `gpt-5.x` or `gemma-4` appear in the response alongside the standard foundation models. ```sh go test ./... ``` Confirm that a failure to reach the Mantle endpoint (e.g. network error, non-200 response) does not prevent the foundation model list from being returned. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations The Mantle request uses the same signing path (`signAWSRequestFromKey`) as other Bedrock requests, or falls back to a Bearer token if a key value is present. No new credentials or secrets are introduced. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable

Summary
Bedrock's
ListFoundationModelsAPI omits models served exclusively through the Mantle OpenAI-compatible endpoint (e.g.gpt-5.x,gemma-4). This PR adds a supplementary call to the Mantle/v1/modelsendpoint and merges the results into the existing foundation model list, ensuring those models are discoverable through Bifrost's list models API.Changes
listMantleModelswhich calls the Bedrock Mantle/v1/modelsendpoint, supporting both Bearer token and AWS SigV4 signing. The function is best-effort and returnsnilon any failure so the foundation model list is always returned.listModelsByKeyresponse is built, the Mantle catalog is merged in, deduplicating by model ID and applying the same allow/blacklist/alias gating as the foundation model path.OpenAIListModelsResponse.ToBifrostListModelsResponseconversion to normalize Mantle model entries into the Bifrost schema.Type of change
Affected areas
How to test
Call the Bifrost list models endpoint against a Bedrock key configured for a region where Mantle models are available and verify that models such as
gpt-5.xorgemma-4appear in the response alongside the standard foundation models.go test ./...Confirm that a failure to reach the Mantle endpoint (e.g. network error, non-200 response) does not prevent the foundation model list from being returned.
Screenshots/Recordings
N/A
Breaking changes
Related issues
N/A
Security considerations
The Mantle request uses the same signing path (
signAWSRequestFromKey) as other Bedrock requests, or falls back to a Bearer token if a key value is present. No new credentials or secrets are introduced.Checklist
docs/contributing/README.mdand followed the guidelines