feat: add per-alias Replicate use deployments override and provider-scoped alias sub-config validation - #4018
Conversation
|
Warning Review limit reached
More reviews will be available in 27 minutes and 31 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 (8)
📝 WalkthroughWalkthroughKeyAliases.Validate now requires a provider argument and validation rejects provider-specific alias sub-configs when the alias provider doesn't match. Replicate's useDeploymentsEndpoint was refactored to accept ctx and prefer a resolved alias's ReplicateAliasCfg.UseDeploymentsEndpoint; all Replicate callers were updated and a unit test was added. ChangesProvider alias validation and Replicate endpoint override
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
|
Confidence Score: 4/5Safe to merge on the Replicate and config-load paths; the GORM hook omission (noted in existing threads) remains an open gap in the database write path but does not affect the new alias-override feature itself. The alias override logic and cross-provider sub-config validation are implemented correctly across the HTTP handler and config-file load paths. The only open concern — the removal of Validate from the GORM BeforeSave hook without replacement — was already identified and discussed in prior review threads. Bulk writes, admin scripts, or any path that bypasses the HTTP handlers can now persist structurally invalid aliases silently. framework/configstore/tables/key.go — the BeforeSave hook no longer validates aliases at all after the signature change. Important Files Changed
Reviews (11): Last reviewed commit: "feat: add replicate alias config support" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
78dfe46 to
49c18dc
Compare
a2efd42 to
129a7ae
Compare
live model cache store and port keyconfig regression tests for alias/model isolation
#4034
129a7ae to
ccbe37d
Compare
49c18dc to
787b2fe
Compare
787b2fe to
8c8ed24
Compare
0ef24e1 to
ddf4e34
Compare
8c8ed24 to
f70d020
Compare
![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
🤖 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/providers/replicate/use_deployments_endpoint_test.go`:
- Around line 13-73: Add a new assertion in
TestUseDeploymentsEndpoint_AliasOverride to cover the documented default when
the key has a nil ReplicateKeyConfig and no alias is present: create a
schemas.Key with ReplicateKeyConfig == nil and call useDeploymentsEndpoint(nil,
keyWithNilConfig) and assert it returns false; reference the existing test setup
(useDeploymentsEndpoint, schemas.Key, ReplicateKeyConfig, and
schemas.NewBifrostContext for pattern) and place this check alongside the other
nil-ctx assertions so the edge-case cannot regress silently.
In `@transports/bifrost-http/lib/config.go`:
- Around line 1243-1246: Validate and normalize
providerCfgInFile.CustomProviderConfig.BaseProviderType before assigning to
baseProvider and using it for alias enforcement: ensure the custom
BaseProviderType is non-empty and is allowed for the owning provider (provider)
at the config-processing site, otherwise reject/ignore it (keep baseProvider =
provider) and surface a validation error; update the assignment and the
alias-check code paths that reference baseProvider so they call this validation
helper (or inline the check) before using baseProvider for alias
validation/enforcement.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: ca7948a8-6468-46c8-b3a5-f35b9b641922
📒 Files selected for processing (8)
core/providers/replicate/replicate.gocore/providers/replicate/replicate_test.gocore/providers/replicate/use_deployments_endpoint_test.gocore/schemas/account.gocore/schemas/account_test.goframework/configstore/tables/key.gotransports/bifrost-http/handlers/provider_keys.gotransports/bifrost-http/lib/config.go
💤 Files with no reviewable changes (1)
- framework/configstore/tables/key.go
f70d020 to
70920f8
Compare
ddf4e34 to
12cdd14
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/schemas/account_test.go`:
- Around line 259-287: The test table in account_test.go contains three
duplicated entries for the KeyAliases validation cases; locate the repeated test
cases that use KeyAliases with ModelID and ModelName (the entries named
"whitespace model_id", "whitespace model_name", and "duplicate source
case-insensitive" that construct KeyAliases{"k": {ModelID: "v "}} /
KeyAliases{"k": {ModelID: "v", ModelName: Ptr(" canonical ")}} /
KeyAliases{"Key": {ModelID: "v"}, "key": {ModelID: "v"}}) and remove the second
occurrence of each duplicate so each case appears only once in the test table.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: b5a4d135-05b5-46ef-9bb1-115ec6892742
📒 Files selected for processing (6)
core/providers/replicate/replicate.gocore/providers/replicate/replicate_test.gocore/providers/replicate/use_deployments_endpoint_test.gocore/schemas/account.gocore/schemas/account_test.goframework/configstore/tables/key.go
💤 Files with no reviewable changes (1)
- framework/configstore/tables/key.go
12cdd14 to
f5d3f4c
Compare
70920f8 to
3d3e263
Compare
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/schemas/account_test.go`:
- Around line 259-272: The three table-driven test entries named "whitespace
model_id", "whitespace model_name", and "duplicate source case-insensitive" omit
the provider and therefore use the zero-value ModelProvider(""); update each
KeyAliases map value in those cases to include provider: OpenAI (or any valid
ModelProvider) so they match the other tests and avoid relying on validation
order (look for the KeyAliases{"k": {ModelID: ...}} and KeyAliases{"Key": {...},
"key": {...}} entries in account_test.go and add provider: OpenAI to those
inline struct literals).
In `@core/schemas/account.go`:
- Around line 205-226: VLLMAliasCfg is defined but never used; either remove it
or wire it into AliasConfig and validation—prefer wiring: embed *VLLMAliasCfg
inside AliasConfig alongside *AzureAliasCfg etc., then update
KeyAliases.Validate(providerKey) to handle providerKey == VLLM (add the branch
that validates the VLLM-specific fields on the alias, mirroring how
AzureAliasCfg/VertexAliasCfg/BedrockAliasCfg/ReplicateAliasCfg are validated) so
the type is reachable and validated.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: c9b8e884-940d-4d27-9cc2-2e0f39c41748
📒 Files selected for processing (8)
core/providers/replicate/replicate.gocore/providers/replicate/replicate_test.gocore/providers/replicate/use_deployments_endpoint_test.gocore/schemas/account.gocore/schemas/account_test.goframework/configstore/tables/key.gotransports/bifrost-http/handlers/provider_keys.gotransports/bifrost-http/lib/config.go
💤 Files with no reviewable changes (1)
- framework/configstore/tables/key.go
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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/schemas/account_test.go`:
- Around line 259-272: The three table-driven test entries named "whitespace
model_id", "whitespace model_name", and "duplicate source case-insensitive" omit
the provider and therefore use the zero-value ModelProvider(""); update each
KeyAliases map value in those cases to include provider: OpenAI (or any valid
ModelProvider) so they match the other tests and avoid relying on validation
order (look for the KeyAliases{"k": {ModelID: ...}} and KeyAliases{"Key": {...},
"key": {...}} entries in account_test.go and add provider: OpenAI to those
inline struct literals).
In `@core/schemas/account.go`:
- Around line 205-226: VLLMAliasCfg is defined but never used; either remove it
or wire it into AliasConfig and validation—prefer wiring: embed *VLLMAliasCfg
inside AliasConfig alongside *AzureAliasCfg etc., then update
KeyAliases.Validate(providerKey) to handle providerKey == VLLM (add the branch
that validates the VLLM-specific fields on the alias, mirroring how
AzureAliasCfg/VertexAliasCfg/BedrockAliasCfg/ReplicateAliasCfg are validated) so
the type is reachable and validated.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: c9b8e884-940d-4d27-9cc2-2e0f39c41748
📒 Files selected for processing (8)
core/providers/replicate/replicate.gocore/providers/replicate/replicate_test.gocore/providers/replicate/use_deployments_endpoint_test.gocore/schemas/account.gocore/schemas/account_test.goframework/configstore/tables/key.gotransports/bifrost-http/handlers/provider_keys.gotransports/bifrost-http/lib/config.go
💤 Files with no reviewable changes (1)
- framework/configstore/tables/key.go
🛑 Comments failed to post (2)
core/schemas/account_test.go (1)
259-272: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
Test cases missing
providerfield use zero-valueModelProvider.The test cases at lines 259-272 ("whitespace model_id", "whitespace model_name", "duplicate source case-insensitive") don't set the
providerfield, so they use the zero-valueModelProvider(""). While these tests will still pass because the validation errors they check occur before the provider sub-config checks, it's inconsistent with other test cases and could mask issues if the validation order changes.Consider adding
provider: OpenAI(or any valid provider) for consistency:♻️ Suggested consistency fix
{ - name: "whitespace model_id", + name: "whitespace model_id", + provider: OpenAI, ka: KeyAliases{"k": {ModelID: "v "}}, wantErr: "model_id cannot have leading or trailing whitespace", }, { - name: "whitespace model_name", + name: "whitespace model_name", + provider: OpenAI, ka: KeyAliases{"k": {ModelID: "v", ModelName: Ptr(" canonical ")}}, wantErr: "model_name cannot have leading or trailing whitespace", }, { - name: "duplicate source case-insensitive", + name: "duplicate source case-insensitive", + provider: OpenAI, ka: KeyAliases{"Key": {ModelID: "v"}, "key": {ModelID: "v"}}, wantErr: "duplicate alias source", },🤖 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/schemas/account_test.go` around lines 259 - 272, The three table-driven test entries named "whitespace model_id", "whitespace model_name", and "duplicate source case-insensitive" omit the provider and therefore use the zero-value ModelProvider(""); update each KeyAliases map value in those cases to include provider: OpenAI (or any valid ModelProvider) so they match the other tests and avoid relying on validation order (look for the KeyAliases{"k": {ModelID: ...}} and KeyAliases{"Key": {...}, "key": {...}} entries in account_test.go and add provider: OpenAI to those inline struct literals).core/schemas/account.go (1)
205-226:
⚠️ Potential issue | 🟡 Minor🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Check if VLLMAliasCfg is used anywhere or if it's intended to be embedded in AliasConfig rg -n "VLLMAliasCfg" --type goRepository: maximhq/bifrost
Length of output: 218
🏁 Script executed:
#!/bin/bash set -euo pipefail file="core/schemas/account.go" echo "== AliasConfig embedding ==" sed -n '180,260p' "$file" | cat -n echo echo "== Validate(providerKey) implementation ==" rg -n "func \\(.*\\) Validate\\(providerKey ModelProvider\\)" "$file" # Print a focused window around the Validate method # (use match line to derive range) line=$(rg -n "func \\(.*\\) Validate\\(providerKey ModelProvider\\)" "$file" | head -n1 | cut -d: -f1) start=$((line-40)) end=$((line+120)) sed -n "${start},${end}p" "$file" | cat -n echo echo "== Search for Validate checks referencing VLLM or VLLMAliasCfg ==" rg -n "VLLM|VLLMAliasCfg|vLLM" "$file"Repository: maximhq/bifrost
Length of output: 13046
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n "ModelProvider.*VLLM|VLLM.*ModelProvider|type ModelProvider|const \\(.*VLLM" core/schemas/bifrost.goRepository: maximhq/bifrost
Length of output: 127
🏁 Script executed:
#!/bin/bash set -euo pipefail file="core/schemas/account.go" echo "== Search for VLLM references in account.go ==" rg -n "VLLM" "$file" echo echo "== Search for providerKey == VLLM usage ==" rg -n "providerKey[[:space:]]*!=|providerKey[[:space:]]*==|==[[:space:]]*VLLM|VLLM" "$file" | head -n 80Repository: maximhq/bifrost
Length of output: 1492
Wire or remove unused
VLLMAliasCfg
core/schemas/account.godefinesVLLMAliasCfg, butAliasConfigonly embeds*AzureAliasCfg,*VertexAliasCfg,*BedrockAliasCfg, and*ReplicateAliasCfg, andKeyAliases.Validate(providerKey)only validates those four—soVLLMAliasCfgis currently unreachable/unused. Either embed*VLLMAliasCfgintoAliasConfigand add theproviderKey == VLLMvalidation, or remove the unused type.🤖 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/schemas/account.go` around lines 205 - 226, VLLMAliasCfg is defined but never used; either remove it or wire it into AliasConfig and validation—prefer wiring: embed *VLLMAliasCfg inside AliasConfig alongside *AzureAliasCfg etc., then update KeyAliases.Validate(providerKey) to handle providerKey == VLLM (add the branch that validates the VLLM-specific fields on the alias, mirroring how AzureAliasCfg/VertexAliasCfg/BedrockAliasCfg/ReplicateAliasCfg are validated) so the type is reachable and validated.
3d3e263 to
481d427
Compare
0173bbf to
7c99fd8
Compare
481d427 to
95c1fa5
Compare
7c99fd8 to
95dedc6
Compare
95c1fa5 to
b81beba
Compare
Merge activity
|
3 participants
Summary
Adds per-alias control over whether Replicate requests are routed to the deployments endpoint or the predictions endpoint. Previously this was only configurable at the key level, meaning all aliases on a single Replicate API token had to use the same endpoint. This change allows individual aliases to override the key-level setting, enabling production-pinned models (deployments) and experimental versioned models (predictions) to coexist under the same key.
Additionally,
KeyAliases.Validatenow accepts the owning key's provider and rejects provider-specific sub-configs (e.g.AzureAliasCfg,BedrockAliasCfg,VertexAliasCfg,ReplicateAliasCfg) when attached to a key belonging to a different provider.Changes
useDeploymentsEndpointnow accepts a*schemas.BifrostContextand checksReplicateAliasCfg.UseDeploymentsEndpointon the resolved alias first, falling back to the key-levelReplicateKeyConfig.UseDeploymentsEndpointwhen the alias override is absent.KeyAliases.Validatenow takes aModelProviderargument and returns an error if a provider-specific alias sub-config is attached to a key from a different provider.KeyAliases.Validate(config file loading, HTTP handler create/update, GORMBeforeSavehook) pass the owning provider.Type of change
Affected areas
How to test
go test ./core/providers/replicate/... ./core/schemas/...To validate the alias override:
use_deployments_endpoint: falseat the key level.replicate_alias_cfg.use_deployments_endpoint: true.To validate cross-provider sub-config rejection:
azure_alias_cfgto a Bedrock key alias and confirm the config fails validation with a descriptive error.Breaking changes
KeyAliases.Validate()now requires aModelProviderargument. Any code callingValidate()directly must be updated to pass the provider the key belongs to.Related issues
Security considerations
None beyond standard config validation.
Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit
New Features
Bug Fixes / Validation
Tests