fix: base provider level request level restriction and updated custom provider docs for go SDK #398
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds provider capability APIs and validation in core schemas; integrates AllowedRequests sanitization into backend create/update flows; updates UI to derive and gate AllowedRequests from base provider capabilities and provider-specific defaults; adds a Go SDK custom-provider example and minor UI/ordering tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Provider Form (UI)
participant API as HTTP Handler
participant Cfg as ValidateCustomProvider
participant Sch as schemas (Get/Validate AllowedRequests)
participant Store as Provider Store
User->>UI: Submit Create/Update Custom Provider
UI->>API: POST /providers {config, allowedRequests, baseProviderType}
API->>Cfg: ValidateCustomProvider(newConfig, req.Provider)
alt allowedRequests provided
Cfg->>Sch: ValidateAllowedRequests(allowedRequests, baseProviderType)
Sch-->>Cfg: sanitized allowedRequests
else none provided
Cfg->>Sch: GetDefaultAllowedRequestsForProvider(baseProviderType)
Sch-->>Cfg: default allowedRequests
end
Cfg-->>API: ok / error
alt validation error
API-->>UI: 400 Bad Request (validation error)
else ok
API->>Store: Add/Update Provider (sanitized allowedRequests)
Store-->>API: ack
API-->>UI: 200 OK
end
sequenceDiagram
autonumber
actor User
participant UI as Provider Form
participant Const as getProviderDefaultAllowedRequests
User->>UI: Select baseProviderType
UI->>Const: getProviderDefaultAllowedRequests(baseProviderType)
Const-->>UI: capability defaults
UI->>UI: Update allowedRequests switches (disable unsupported)
UI-->>User: Render gated controls and hint text
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
TejasGhatte
changed the title
TejasGhatte
changed the title
TejasGhatte
changed the title
TejasGhatte
force-pushed
the
09-01-fix_base_provider_level_request_level_restriction
branch
from
6d784ea to
90b6637
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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
transports/bifrost-http/lib/config.go (2)
976-979: Update path misses AllowedRequests sanitization.
ValidateCustomProviderUpdatedoesn't sanitizeAllowedRequests, so updates can re-enable unsupported ops. Sanitize here too to keep behavior consistent with creation.Add inside
ValidateCustomProviderUpdateafter the base type immutability check:// Normalize/sanitize AllowedRequests on update as well if newCPC.AllowedRequests == nil { caps := schemas.GetProviderCapabilities(newCPC.BaseProviderType) newCPC.AllowedRequests = &schemas.AllowedRequests{ TextCompletion: caps.TextCompletion, ChatCompletion: caps.ChatCompletion, ChatCompletionStream: caps.ChatCompletionStream, Embedding: caps.Embedding, Speech: caps.Speech, SpeechStream: caps.SpeechStream, Transcription: caps.Transcription, TranscriptionStream: caps.TranscriptionStream, } } else { schemas.ValidateAllowedRequests(newCPC.AllowedRequests, newCPC.BaseProviderType) }Optionally log when the incoming flags are narrowed to aid debugging.
966-1047: Consider last-mile safety: sanitize before persisting.Even with the fixes above, adding a final guard before
s.Providers[provider] = configavoids future regressions.Insert just before assigning
s.Providers[provider]:if config.CustomProviderConfig != nil && config.CustomProviderConfig.AllowedRequests != nil { schemas.ValidateAllowedRequests(config.CustomProviderConfig.AllowedRequests, config.CustomProviderConfig.BaseProviderType) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
core/schemas/provider.go(1 hunks)docs/features/custom-providers.mdx(1 hunks)framework/go.mod(1 hunks)transports/bifrost-http/handlers/providers.go(1 hunks)transports/bifrost-http/lib/config.go(1 hunks)transports/go.mod(1 hunks)ui/app/providers/configure/provider-form.tsx(15 hunks)ui/lib/constants/config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
transports/bifrost-http/lib/config.go (2)
core/schemas/provider.go (2)
AllowedRequests(87-96)ValidateAllowedRequests(202-235)ui/lib/types/config.ts (1)
AllowedRequests(88-97)
transports/bifrost-http/handlers/providers.go (2)
core/schemas/provider.go (3)
CustomProviderConfig(237-241)AllowedRequests(87-96)ValidateAllowedRequests(202-235)ui/lib/types/config.ts (2)
CustomProviderConfig(82-85)AllowedRequests(88-97)
ui/lib/constants/config.ts (2)
core/schemas/provider.go (1)
AllowedRequests(87-96)ui/lib/types/config.ts (1)
AllowedRequests(88-97)
core/schemas/provider.go (2)
core/schemas/bifrost.go (5)
ModelProvider(38-38)OpenAI(41-41)Anthropic(43-43)Cohere(45-45)Bedrock(44-44)ui/lib/types/config.ts (2)
ModelProvider(12-12)AllowedRequests(88-97)
ui/app/providers/configure/provider-form.tsx (2)
ui/lib/constants/config.ts (1)
getProviderDefaultAllowedRequests(86-101)ui/lib/utils/validation.ts (1)
Validator(40-151)
🔇 Additional comments (5)
ui/app/providers/configure/provider-form.tsx (3)
426-434: Good fix for duplicate name validationThe logic correctly prevents duplicate name validation when updating existing providers, which resolves a common UX issue where users couldn't save their own provider during updates.
864-867: Clear and helpful UI text for disabled featuresThe explanatory text effectively communicates to users why certain features are disabled based on the underlying provider's capabilities. This improves the user experience by setting clear expectations.
877-943: Consistent capability gating implementationThe switch controls are properly disabled based on the provider's capabilities using
getProviderDefaultAllowedRequests. This ensures users cannot enable unsupported operations, preventing runtime errors.transports/bifrost-http/lib/config.go (1)
590-591: Verify EnvKeyInfo.ConfigPath vs getRestoredMCPConfig mapping
Ensure the paths used ingetRestoredMCPConfig(e.g.mcp.client_configs[%d].connection_string) align with howEnvKeyInfo.ConfigPathis stored (mcp.client_configs.<name>.connection_string); otherwise restoration may skip entries and leak values.core/schemas/provider.go (1)
126-136: LGTM on the capabilities type.Clear, minimal surface; pairs cleanly with validation and UI.
TejasGhatte
force-pushed
the
09-01-fix_base_provider_level_request_level_restriction
branch
from
90b6637 to
6507562
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
docs/features/custom-providers.mdx (1)
24-29: Doc behavior inconsistent with backend: nil allowed_requests defaults to provider capabilities, not “all allowed”Update this section to reflect current CustomProviderConfig.IsOperationAllowed semantics.
-**Important**: The `allowed_requests` field follows a specific behavior: -- **Omitted entirely**: All operations are allowed (default behavior) -- **Partially specified**: Only explicitly set fields are allowed, others default to `false` -- **Fully specified**: Only the operations you explicitly enable are allowed -- **Present but empty object (`{}`)**: All fields are set to false +**Important**: The `allowed_requests` field follows this behavior: +- **Omitted entirely**: Defaults to the capabilities of the selected base provider (unsupported operations remain disabled) +- **Partially specified**: Only explicitly set fields are allowed; omitted fields default to `false` and are further limited by base provider capabilities +- **Fully specified**: Only the operations you explicitly enable are allowed, limited by base provider capabilities +- **Present but empty object (`{}`)**: All fields are set to `false`ui/app/providers/configure/provider-form.tsx (3)
187-189: Keys/base URL requirements should depend on effectiveProviderType (bug blocks custom Ollama/SGL).Currently keyed to selectedProvider, so custom providers based on ollama/sgl incorrectly require keys and don’t require base_url. Use effectiveProviderType.
- const baseURLRequired = selectedProvider === "ollama" || selectedProvider === "sgl"; - const keysRequired = selectedProvider === "custom" || !["ollama", "sgl"].includes(selectedProvider); // Custom providers and most others need keys + const baseURLRequired = effectiveProviderType === "ollama" || effectiveProviderType === "sgl"; + const keysRequired = !["ollama", "sgl"].includes(effectiveProviderType);
870-946: Avoid repeated calls to getProviderDefaultAllowedRequests in disabled checks.Memoize baseCaps once and reference it. Reduces rework on each render.
Add near other useMemo declarations:
const baseCaps = useMemo( () => getProviderDefaultAllowedRequests(baseProviderType || "openai"), [baseProviderType] );Then update switches:
- disabled={!getProviderDefaultAllowedRequests(baseProviderType || "openai").text_completion} + disabled={!baseCaps.text_completion} ... - disabled={!getProviderDefaultAllowedRequests(baseProviderType || "openai").chat_completion} + disabled={!baseCaps.chat_completion} ... - disabled={!getProviderDefaultAllowedRequests(baseProviderType || "openai").chat_completion_stream} + disabled={!baseCaps.chat_completion_stream} ... - disabled={!getProviderDefaultAllowedRequests(baseProviderType || "openai").embedding} + disabled={!baseCaps.embedding} ... - disabled={!getProviderDefaultAllowedRequests(baseProviderType || "openai").speech} + disabled={!baseCaps.speech} ... - disabled={!getProviderDefaultAllowedRequests(baseProviderType || "openai").speech_stream} + disabled={!baseCaps.speech_stream} ... - disabled={!getProviderDefaultAllowedRequests(baseProviderType || "openai").transcription} + disabled={!baseCaps.transcription} ... - disabled={!getProviderDefaultAllowedRequests(baseProviderType || "openai").transcription_stream} + disabled={!baseCaps.transcription_stream}
1094-1136: Vertex key UI is hidden for custom Vertex providers (wrong condition).Gate on effectiveProviderType, not selectedProvider, else custom providers based on Vertex can’t configure required fields.
- {selectedProvider === "vertex" && ( + {effectiveProviderType === "vertex" && (transports/bifrost-http/lib/config.go (1)
410-474: Also validate/sanitize custom providers when loading from config.json (parity with DB path).This branch processes providers from the file but doesn’t call ValidateCustomProvider before assigning cfg, so unsupported ops could slip through until first update.
- processedProviders[provider] = cfg + if err := ValidateCustomProvider(cfg, provider); err != nil { + logger.Warn("invalid custom provider config for %s: %v", provider, err) + continue + } + processedProviders[provider] = cfg
♻️ Duplicate comments (2)
ui/lib/constants/config.ts (1)
33-83: Reduce duplication: generate provider defaults from a compact capability setThis manually repeated boolean matrix is brittle and easy to drift from core. Generate it from a single capability set.
-// Define the default allowed requests for each provider type -// This is based on the actual capabilities of each provider -export const PROVIDER_DEFAULT_ALLOWED_REQUESTS: Record<string, AllowedRequests> = { - // OpenAI - openai: { - text_completion: false, - chat_completion: true, - chat_completion_stream: true, - embedding: true, - speech: true, - speech_stream: true, - transcription: true, - transcription_stream: true, - }, - // Anthropic - anthropic: { - text_completion: true, - chat_completion: true, - chat_completion_stream: true, - embedding: false, - speech: false, - speech_stream: false, - transcription: false, - transcription_stream: false, - }, - // Cohere - cohere: { - text_completion: false, - chat_completion: true, - chat_completion_stream: true, - embedding: true, - speech: false, - speech_stream: false, - transcription: false, - transcription_stream: false, - }, - // AWS Bedrock - bedrock: { - text_completion: true, - chat_completion: true, - chat_completion_stream: true, - embedding: true, - speech: false, - speech_stream: false, - transcription: false, - transcription_stream: false, - }, -}; +type ProviderCapability = + | "text_completion" + | "chat_completion" + | "chat_completion_stream" + | "embedding" + | "speech" + | "speech_stream" + | "transcription" + | "transcription_stream"; + +const PROVIDER_CAPABILITIES: Record<string, ReadonlySet<ProviderCapability>> = { + openai: new Set([ + "chat_completion", + "chat_completion_stream", + "embedding", + "speech", + "speech_stream", + "transcription", + "transcription_stream", + ]), + anthropic: new Set(["text_completion", "chat_completion", "chat_completion_stream"]), + cohere: new Set(["chat_completion", "chat_completion_stream", "embedding"]), + bedrock: new Set(["text_completion", "chat_completion", "chat_completion_stream", "embedding"]), +} as const; + +const ALL_CAPS: ProviderCapability[] = [ + "text_completion", + "chat_completion", + "chat_completion_stream", + "embedding", + "speech", + "speech_stream", + "transcription", + "transcription_stream", +]; + +export const PROVIDER_DEFAULT_ALLOWED_REQUESTS: Record<string, AllowedRequests> = Object.fromEntries( + Object.entries(PROVIDER_CAPABILITIES).map(([k, caps]) => [ + k, + Object.fromEntries( + ALL_CAPS.map((c) => [c, caps.has(c)]) + ) as AllowedRequests, + ]) +);ui/app/providers/configure/provider-form.tsx (1)
193-204: Debounce/cancel the allowedRequests reset to avoid flicker and stale updates.-useEffect(() => { - if (baseProviderType && isCustomProvider && !isEditingExisting) { - const newAllowedRequests = getProviderDefaultAllowedRequests(baseProviderType); - setFormData((prev) => ({ - ...prev, - allowedRequests: newAllowedRequests, - isDirty: true, - })); - } -}, [baseProviderType, isCustomProvider, isEditingExisting]); +useEffect(() => { + if (!baseProviderType || !isCustomProvider || isEditingExisting) return; + const id = setTimeout(() => { + const newAllowedRequests = getProviderDefaultAllowedRequests(baseProviderType); + setFormData((prev) => ({ + ...prev, + allowedRequests: newAllowedRequests, + isDirty: true, + })); + }, 200); + return () => clearTimeout(id); +}, [baseProviderType, isCustomProvider, isEditingExisting]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
core/schemas/provider.go(2 hunks)docs/features/custom-providers.mdx(1 hunks)transports/bifrost-http/handlers/providers.go(1 hunks)transports/bifrost-http/lib/config.go(2 hunks)ui/app/providers/configure/provider-form.tsx(15 hunks)ui/lib/constants/config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
ui/lib/constants/config.ts (2)
core/schemas/provider.go (1)
AllowedRequests(87-96)ui/lib/types/config.ts (1)
AllowedRequests(88-97)
transports/bifrost-http/lib/config.go (2)
core/schemas/provider.go (3)
AllowedRequests(87-96)ValidateAllowedRequests(197-230)GetDefaultAllowedRequestsForProvider(190-193)ui/lib/types/config.ts (1)
AllowedRequests(88-97)
core/schemas/provider.go (2)
core/schemas/bifrost.go (5)
ModelProvider(38-38)OpenAI(41-41)Anthropic(43-43)Cohere(45-45)Bedrock(44-44)ui/lib/types/config.ts (2)
ModelProvider(12-12)AllowedRequests(88-97)
transports/bifrost-http/handlers/providers.go (3)
core/schemas/provider.go (1)
CustomProviderConfig(232-236)transports/bifrost-http/lib/config.go (1)
ValidateCustomProvider(1824-1855)transports/bifrost-http/handlers/utils.go (1)
SendError(27-36)
ui/app/providers/configure/provider-form.tsx (2)
ui/lib/constants/config.ts (1)
getProviderDefaultAllowedRequests(86-101)ui/lib/utils/validation.ts (1)
Validator(40-151)
🔇 Additional comments (5)
docs/features/custom-providers.mdx (1)
112-174: Verify SDK interface signaturesPlease confirm the account interface expects GetKeysForProvider(ctx context.Context, …) and not a pointer. If it does require a pointer, adjust imports and examples consistently.
ui/app/providers/configure/provider-form.tsx (3)
20-20: Import looks correct.
427-435: Scoped duplicate-name validation only on create — good.
864-867: Helpful UX copy.transports/bifrost-http/lib/config.go (1)
1845-1852: Good: normalize/sanitize AllowedRequests on create.Defaults to base capabilities when nil; otherwise sanitizes in place. Aligns backend with UI gating.
TejasGhatte
force-pushed
the
09-01-fix_base_provider_level_request_level_restriction
branch
from
6507562 to
fb5f099
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
core/schemas/provider.go (2)
126-177: Centralize capability definitions to reduce duplication and drift.Prefer a package-level map[ModelProvider]AllowedRequests + lookup over a switch; easier to keep UI defaults and backend in sync. Also add/keep a small table-driven test to assert UI defaults == core capabilities.
Example sketch:
+var providerCapabilities = map[ModelProvider]AllowedRequests{ + OpenAI: {ChatCompletion: true, ChatCompletionStream: true, Embedding: true, Speech: true, SpeechStream: true, Transcription: true, TranscriptionStream: true}, + Anthropic: {TextCompletion: true, ChatCompletion: true, ChatCompletionStream: true}, + Cohere: {ChatCompletion: true, ChatCompletionStream: true, Embedding: true}, + Bedrock: {TextCompletion: true, ChatCompletion: true, ChatCompletionStream: true, Embedding: true}, +} func GetProviderCapabilities(provider ModelProvider) AllowedRequests { - switch provider { - ... - default: - return AllowedRequests{} - } + if caps, ok := providerCapabilities[provider]; ok { + return caps + } + return AllowedRequests{} }
186-221: Document in-place mutation and offer a pure sanitizer to avoid surprises.This function mutates its input; call-sites must pass pointers they own. Either document this explicitly or provide a pure alternative returning a sanitized copy.
Apply:
-// ValidateAllowedRequests checks if the allowed requests are valid for the given base provider -// and silently sets any unsupported operations to false instead of returning an error +// ValidateAllowedRequests mutates allowedRequests in place to disable unsupported operations +// for the given base provider. Unsupported true flags are silently set to false. func ValidateAllowedRequests(allowedRequests *AllowedRequests, baseProvider ModelProvider) { ... } + +// SanitizeAllowedRequests returns a new AllowedRequests with unsupported ops disabled. +func SanitizeAllowedRequests(in *AllowedRequests, baseProvider ModelProvider) *AllowedRequests { + if in == nil { + return nil + } + out := *in + ValidateAllowedRequests(&out, baseProvider) + return &out +}
🧹 Nitpick comments (3)
ui/app/providers/configure/page.tsx (1)
14-23: Gate auto-select to avoid premature dispatch and accidental “create-new” override.Add an isSuccess guard (from RTK Query) and trim the deps to reduce re-runs. This avoids dispatching before data is ready and prevents auto-select from hijacking a create-new flow when selectedProvider is intentionally unset.
- Confirm: is there a separate “create new provider” path that relies on selectedProvider being empty? If yes, consider gating this effect behind a route flag or explicit “edit” mode.
Apply:
- useEffect(() => { - if (!selectedProvider && providersData?.providers && providersData.providers.length > 0) { + useEffect(() => { + if (!selectedProvider && providersData?.providers?.length) { const providerToSelect = providersData.providers[0]; // Update Redux state to select this provider dispatch(setSelectedProvider(providerToSelect)); } - }, [selectedProvider, providersData?.providers, dispatch]); + }, [selectedProvider, providersData?.providers?.length, dispatch]);And also destructure isSuccess (keep usage inline if you prefer):
// near Line 12 const { data: providersData, refetch, isSuccess } = useGetProvidersQuery();Optionally tighten the condition further:
- if (!selectedProvider && providersData?.providers?.length) { + if (!selectedProvider && isSuccess && providersData?.providers?.length) {Nit: if provider ordering isn’t deterministic, consider sorting before picking index 0 to avoid UI surprises.
core/schemas/provider.go (1)
231-238: Clarify nil semantics for CustomProviderConfig.AllowedRequests.Make the “not allow-all” behavior explicit to avoid confusion with AllowedRequests’ general nil semantics.
- if cpc.AllowedRequests == nil { - // Use provider capabilities as default when no explicit restrictions are set + if cpc.AllowedRequests == nil { + // Note: When AllowedRequests is nil on CustomProviderConfig, we default to the + // base provider's capabilities (NOT "allow all"). This keeps unsupported ops disabled + // even without explicit restrictions. defaultAllowedRequests := GetDefaultAllowedRequestsForProvider(cpc.BaseProviderType) return defaultAllowedRequests.IsOperationAllowed(operation) }transports/bifrost-http/lib/config.go (1)
1854-1872: Good normalization of AllowedRequests; minor dedupe.Logic aligns with PR goals. You can simplify the nil branch by reusing the helper.
- caps := schemas.GetProviderCapabilities(cpc.BaseProviderType) - cpc.AllowedRequests = &schemas.AllowedRequests{ - TextCompletion: caps.TextCompletion, - ChatCompletion: caps.ChatCompletion, - ChatCompletionStream: caps.ChatCompletionStream, - Embedding: caps.Embedding, - Speech: caps.Speech, - SpeechStream: caps.SpeechStream, - Transcription: caps.Transcription, - TranscriptionStream: caps.TranscriptionStream, - } + cpc.AllowedRequests = schemas.GetDefaultAllowedRequestsForProvider(cpc.BaseProviderType)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
core/schemas/provider.go(2 hunks)docs/features/custom-providers.mdx(1 hunks)transports/bifrost-http/handlers/providers.go(1 hunks)transports/bifrost-http/lib/config.go(2 hunks)ui/app/providers/configure/page.tsx(2 hunks)ui/app/providers/configure/provider-form.tsx(15 hunks)ui/app/providers/views/providers-list.tsx(1 hunks)ui/lib/constants/config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- ui/app/providers/views/providers-list.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- ui/lib/constants/config.ts
- transports/bifrost-http/handlers/providers.go
- docs/features/custom-providers.mdx
- ui/app/providers/configure/provider-form.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
transports/bifrost-http/lib/config.go (1)
core/schemas/provider.go (3)
AllowedRequests(87-96)ValidateAllowedRequests(188-221)GetProviderCapabilities(127-177)
core/schemas/provider.go (2)
core/schemas/bifrost.go (5)
ModelProvider(38-38)OpenAI(41-41)Anthropic(43-43)Cohere(45-45)Bedrock(44-44)ui/lib/types/config.ts (2)
ModelProvider(12-12)AllowedRequests(88-97)
🔇 Additional comments (3)
ui/app/providers/configure/page.tsx (1)
3-5: LGTM — imports and hook usage look correct.core/schemas/provider.go (1)
179-185: LGTM — clear helper returning provider defaults.transports/bifrost-http/lib/config.go (1)
1905-1907: LGTM — update path reuses creation validation (consistent sanitization).
TejasGhatte
force-pushed
the
09-01-fix_base_provider_level_request_level_restriction
branch
from
fb5f099 to
e5ac17c
Compare
TejasGhatte
force-pushed
the
09-01-fix_base_provider_level_request_level_restriction
branch
from
e5ac17c to
af1820c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ui/lib/constants/config.ts (1)
97-101: Narrow lookup key; reuse shared fallback (repeat of prior suggestion).Use
keyof typeof PROVIDER_DEFAULT_ALLOWED_REQUESTSfor the normalized key to gain type safety.-export const getProviderDefaultAllowedRequests = (providerName: string): AllowedRequests => { - const normalizedName = providerName.toLowerCase().trim(); - return PROVIDER_DEFAULT_ALLOWED_REQUESTS[normalizedName] ?? (DEFAULT_ALLOWED_REQUESTS as AllowedRequests); -}; +export const getProviderDefaultAllowedRequests = (providerName: string): AllowedRequests => { + const key = providerName.toLowerCase().trim() as keyof typeof PROVIDER_DEFAULT_ALLOWED_REQUESTS; + return PROVIDER_DEFAULT_ALLOWED_REQUESTS[key] ?? (DEFAULT_ALLOWED_REQUESTS as AllowedRequests); +};
🧹 Nitpick comments (1)
ui/lib/constants/config.ts (1)
35-95: Type-safe provider map; cover all known providers to prevent drift.
- Type the map against
Providerto eliminate key typos and make intent explicit.- Using
Partial<Record<Provider,...>>keeps it flexible while still tied to the union.- Consider adding entries for azure, cerebras, groq, mistral, ollama, parasail, sgl, vertex to avoid the all-true fallback UX for those bases.
Apply:
+// Tie defaults to the Provider union to avoid key drift/typos +import type { Provider } from "@/lib/constants/logs"; @@ -export const PROVIDER_DEFAULT_ALLOWED_REQUESTS: Record<string, AllowedRequests> = { +export const PROVIDER_DEFAULT_ALLOWED_REQUESTS = { // OpenAI openai: { @@ // Gemini gemini: { @@ -}; +} as const satisfies Partial<Record<Provider, AllowedRequests>>;If desired, add stubs (with correct booleans from core) for the remaining providers:
// azure: { /* fill from core GetProviderCapabilities */ }, // cerebras: { /* ... */ }, // groq: { /* ... */ }, // mistral: { /* ... */ }, // ollama: { /* ... */ }, // parasail: { /* ... */ }, // sgl: { /* ... */ }, // vertex: { /* ... */ },To keep UI in lockstep with core, extend the existing guardrail to include all providers present in
KNOWN_PROVIDERS, failing on any mismatch. I can wire this up if you want.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
core/schemas/provider.go(2 hunks)docs/features/custom-providers.mdx(1 hunks)transports/bifrost-http/handlers/providers.go(1 hunks)transports/bifrost-http/lib/config.go(2 hunks)ui/app/providers/configure/page.tsx(2 hunks)ui/app/providers/configure/provider-form.tsx(15 hunks)ui/app/providers/views/providers-list.tsx(1 hunks)ui/lib/constants/config.ts(1 hunks)ui/lib/constants/logs.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- transports/bifrost-http/handlers/providers.go
- ui/app/providers/views/providers-list.tsx
- transports/bifrost-http/lib/config.go
- ui/app/providers/configure/page.tsx
- docs/features/custom-providers.mdx
- core/schemas/provider.go
- ui/app/providers/configure/provider-form.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
ui/lib/constants/config.ts (2)
core/schemas/provider.go (1)
AllowedRequests(87-96)ui/lib/types/config.ts (1)
AllowedRequests(88-97)
🔇 Additional comments (1)
ui/lib/constants/config.ts (1)
32-32: Good: strong shape guarantee for defaults.
satisfies Required<AllowedRequests>is the right guard to keep the constant in sync with the interface.
| } | ||
|
|
||
| // GetProviderCapabilities returns the capabilities for a given provider | ||
| func GetProviderCapabilities(provider ModelProvider) AllowedRequests { |
There was a problem hiding this comment.
@TejasGhatte pls change its name to get GetBaseProviderCapabilities cause it only contains the base providers, not all
Provider Capabilities and Validation for Custom Providers
Added a new system to define and validate provider capabilities, ensuring custom providers only enable operations supported by their base provider type.
Changes
ProviderCapabilitiesstruct to define what operations each provider supportsGetProviderCapabilitiesfunction to return capabilities for each provider typeValidateAllowedRequestsfunction to automatically disable unsupported operationsType of change
Affected areas
How to test
Breaking changes
Security considerations
This change improves system integrity by preventing custom providers from attempting operations not supported by their base provider.
Checklist