Conversation
|
Caution Review failedThe pull request is closed. Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe changes introduce configurable base URL support for OpenAI, Anthropic, and Cohere providers by adding a Changes
Sequence Diagram(s)sequenceDiagram
participant Config as ProviderConfig
participant Provider as Provider (OpenAI/Anthropic/Cohere)
participant API as API Endpoint
Config->>Provider: Initialize with NetworkConfig.BaseURL
Provider->>Provider: Set baseURL (default if empty)
User->>Provider: ChatCompletion/TextCompletion request
Provider->>API: Send request to baseURL + endpoint path
API-->>Provider: Return response
Provider-->>User: Return completion result
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
✨ Finishing Touches
🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (2)
core/schemas/provider.go (1)
30-35: 🧹 Nitpick (assertive)Comment style & serialization nitpick
- Missing space after the
//in the field comment (//BaseURL→// BaseURL).- Consider adding
omitemptyto the tag to avoid serialising empty strings:- //BaseURL is only supported for OpenAI, Anthropic and Cohere providers - BaseURL string `json:"base_url"` // Base URL for the provider (optional) + // BaseURL is only supported for OpenAI, Anthropic and Cohere providers. + BaseURL string `json:"base_url,omitempty"` // Base URL for the provider (optional)This keeps JSON payloads clean and stays consistent with the other optional fields.
core/providers/cohere.go (1)
349-353: 🧹 Nitpick (assertive)Minor:
fmt.Sprintfmakes the path clearerPurely readability; avoid manual concatenation:
-req.SetRequestURI(provider.baseURL + "/v1/chat") +req.SetRequestURI(fmt.Sprintf("%s/v1/chat", provider.baseURL))Up to you – not blocking.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
core/providers/anthropic.go(5 hunks)core/providers/cohere.go(3 hunks)core/providers/openai.go(4 hunks)core/schemas/provider.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
core/providers/anthropic.go (2)
core/schemas/logger.go (1)
Logger(18-35)core/schemas/provider.go (1)
NetworkConfig(29-36)
core/providers/cohere.go (2)
core/schemas/logger.go (1)
Logger(18-35)core/schemas/provider.go (1)
NetworkConfig(29-36)
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ BugBot reviewed your changes and found no bugs!
BugBot free trial expires on June 17, 2025
You have used $0.00 of your $50.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
Pratham-Mishra04
force-pushed
the
06-13-enhancement_base_url_support_added_to_providers
branch
2 times, most recently
from
29ce962 to
0ea6031
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (5)
core/providers/cohere.go (1)
115-119:⚠️ Potential issueCompilation blocker – cannot range over an
int
for range config.ConcurrencyAndBufferSize.Concurrency { … }will not compile (cannot range over Concurrency).
Replace with an indexed loop:- for range config.ConcurrencyAndBufferSize.Concurrency { + for i := 0; i < config.ConcurrencyAndBufferSize.Concurrency; i++ { cohereResponsePool.Put(&CohereChatResponse{}) bifrostResponsePool.Put(&schemas.BifrostResponse{}) }core/providers/anthropic.go (1)
141-145:⚠️ Potential issueCompilation blocker – cannot range over an
intSame issue as in Cohere provider.
- for range config.ConcurrencyAndBufferSize.Concurrency { + for i := 0; i < config.ConcurrencyAndBufferSize.Concurrency; i++ { anthropicTextResponsePool.Put(&AnthropicTextResponse{}) anthropicChatResponsePool.Put(&AnthropicChatResponse{}) bifrostResponsePool.Put(&schemas.BifrostResponse{}) }core/providers/openai.go (1)
86-90:⚠️ Potential issueCompilation blocker – cannot range over an
int- for range config.ConcurrencyAndBufferSize.Concurrency { + for i := 0; i < config.ConcurrencyAndBufferSize.Concurrency; i++ { openAIResponsePool.Put(&OpenAIResponse{}) bifrostResponsePool.Put(&schemas.BifrostResponse{}) }core/bifrost.go (2)
351-358:⚠️ Potential issue
minis undefined – compilation will fail
minis not part of the standard-library imports and no helper with that name exists in this file.
The call therefore causes an “undefined: min” compiler error.- backoff := min(config.NetworkConfig.RetryBackoffInitial*time.Duration(1<<uint(attempt)), config.NetworkConfig.RetryBackoffMax) + // Helper to choose the smaller duration + minDur := func(a, b time.Duration) time.Duration { + if a < b { + return a + } + return b + } + backoff := minDur(config.NetworkConfig.RetryBackoffInitial*time.Duration(1<<uint(attempt)), + config.NetworkConfig.RetryBackoffMax)Add the helper locally (or move it to a shared util) to restore buildability.
633-639: 🛠️ Refactor suggestionIncorrect PostHook count – may execute hooks that never ran a PreHook
RunPostHooksshould receive only the number of plugins whosePreHookactually ran (preCount).
Passinglen(bifrost.plugins)risks calling a PostHook whose PreHook short-circuited or errored out earlier.- resp, bifrostErr := pipeline.RunPostHooks(&ctx, result, nil, len(bifrost.plugins)) + resp, bifrostErr := pipeline.RunPostHooks(&ctx, result, nil, preCount)Repeat the same change for the error branch below.
Identical issue exists intryChatCompletion(lines 757-763 & 765-772).This keeps the Pre/Post symmetry the pipeline contract guarantees.
Also applies to: 645-652
♻️ Duplicate comments (3)
core/providers/cohere.go (1)
121-124: Base-URL normalisation repeated in every provider – consider a small helper (normalizeBaseURL) and/or embedding a common struct to DRY this up.core/providers/anthropic.go (1)
150-160: API version hard-coded – still fixed to"2023-06-01". Exposing this via config (or env var) was discussed earlier; keeping it hard-coded forces a re-compile for every Anthropic upgrade.core/providers/openai.go (1)
94-98: Repeated Base-URL trimming logic – consider extracting to a shared helper to avoid subtle drift between providers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
core/bifrost.go(8 hunks)core/providers/anthropic.go(5 hunks)core/providers/cohere.go(3 hunks)core/providers/openai.go(4 hunks)core/schemas/plugin.go(2 hunks)core/schemas/provider.go(1 hunks)docs/plugins.md(9 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
core/providers/anthropic.go (2)
core/schemas/logger.go (1)
Logger(18-35)core/schemas/provider.go (1)
NetworkConfig(29-36)
🪛 LanguageTool
docs/plugins.md
[uncategorized] ~172-~172: You might be missing the article “the” here.
Context: ...peline. - To invalidate a response, set result to nil and return a non-nil err. - Keep...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (1)
core/schemas/plugin.go (1)
19-27: Comments read clearly – no further action required
Only documentation was touched; wording accurately reflects the current implementation of the plugin pipeline.
Pratham-Mishra04
changed the title
Pratham-Mishra04
changed the title
Pratham-Mishra04
force-pushed
the
06-13-enhancement_base_url_support_added_to_providers
branch
from
0ea6031 to
11585c7
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (4)
core/providers/cohere.go (2)
115-124:⚠️ Potential issue
for range <int>is a compile-time error
rangecannot iterate over anint; this will not build.-// Pre-warm response pools -for range config.ConcurrencyAndBufferSize.Concurrency { +// Pre-warm response pools +for i := 0; i < config.ConcurrencyAndBufferSize.Concurrency; i++ {Apply the same fix in other providers that copied this pattern.
349-353: 🧹 Nitpick (assertive)Robust URI construction
String concatenation can mis-behave if users pass a path segment in
base_url.
Preferurl.JoinPath(Go 1.19+) for safety:-req.SetRequestURI(provider.baseURL + "/v1/chat") +req.SetRequestURI(url.JoinPath(provider.baseURL, "v1", "chat"))core/providers/openai.go (1)
150-154: 🧹 Nitpick (assertive)Join paths instead of concatenation
-req.SetRequestURI(provider.baseURL + "/v1/chat/completions") +req.SetRequestURI(url.JoinPath(provider.baseURL, "v1", "chat", "completions"))Prevents accidental double slashes or duplicate
/v1segments.core/bifrost.go (1)
633-641: 🧹 Nitpick (assertive)Avoid re-shadowing
respwith:=—use assignment for consistencyInside both
selectblocks a newrespvariable is created with:=, shadowing the previously declaredresp. This is harmless but inconsistent (the error branch already uses=).- resp, bifrostErr := pipeline.RunPostHooks(&ctx, result, nil, len(bifrost.plugins)) + resp, bifrostErr = pipeline.RunPostHooks(&ctx, result, nil, len(bifrost.plugins))Replicate the same change in
tryChatCompletion.This tiny tweak eliminates unnecessary shadowing and keeps both branches symmetrical.
Also applies to: 755-761
♻️ Duplicate comments (2)
core/providers/anthropic.go (2)
150-154: Base-URL trimming duplication
Same note as Cohere/OpenAI – a shared helper would remove repetition.
159-160:apiVersionstill hard-codedPrevious feedback about making the version configurable hasn’t been addressed.
Consider exposing it via config or env var so users can upgrade without recompiling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
core/bifrost.go(8 hunks)core/providers/anthropic.go(5 hunks)core/providers/cohere.go(3 hunks)core/providers/openai.go(4 hunks)core/schemas/plugin.go(2 hunks)core/schemas/provider.go(1 hunks)docs/plugins.md(9 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
core/providers/anthropic.go (2)
core/schemas/logger.go (1)
Logger(18-35)core/schemas/provider.go (1)
NetworkConfig(29-36)
core/providers/openai.go (2)
core/schemas/logger.go (1)
Logger(18-35)core/schemas/provider.go (1)
NetworkConfig(29-36)
core/bifrost.go (3)
core/schemas/plugin.go (1)
Plugin(29-50)core/schemas/logger.go (1)
Logger(18-35)core/schemas/bifrost.go (2)
BifrostResponse(237-247)BifrostError(365-371)
core/providers/cohere.go (2)
core/schemas/logger.go (1)
Logger(18-35)core/schemas/provider.go (1)
NetworkConfig(29-36)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (2)
core/schemas/provider.go (1)
30-31: Field tag polish looks goodSpace after
//andomitemptyflag correctly added. No further concerns.core/schemas/plugin.go (1)
12-28: Documentation update is clear & thoroughThe expanded comments accurately describe pipeline behaviour and edge-cases.
No action needed.
Pratham-Mishra04
changed the base branch from
06-13-feat_plugin_handling_in_bifrost_enchancements
to
graphite-base/79
Pratham-Mishra04
force-pushed
the
06-13-enhancement_base_url_support_added_to_providers
branch
from
11585c7 to
43e879a
Compare
akshaydeo
deleted the
06-13-enhancement_base_url_support_added_to_providers
branch
) # Add Custom Base URL Support for OpenAI, Anthropic, and Cohere Providers This PR adds support for configuring custom base URLs for the OpenAI, Anthropic, and Cohere providers. This enhancement allows users to: - Connect to alternative API endpoints or self-hosted instances - Use compatible API proxies - Support enterprise deployments with custom domains The implementation: - Adds a `base_url` field to the `NetworkConfig` struct - Modifies each provider to use the configured base URL or fall back to the default if not specified - Ensures proper URL handling by trimming trailing slashes from configured URLs - Maintains the API version for Anthropic as a provider property For Anthropic, the default remains `https://api.anthropic.com`, for OpenAI it's `https://api.openai.com`, and for Cohere it's `https://api.cohere.ai`.
2 participants
Add Custom Base URL Support for OpenAI, Anthropic, and Cohere Providers
This PR adds support for configuring custom base URLs for the OpenAI, Anthropic, and Cohere providers. This enhancement allows users to:
The implementation:
base_urlfield to theNetworkConfigstructFor Anthropic, the default remains
https://api.anthropic.com, for OpenAI it'shttps://api.openai.com, and for Cohere it'shttps://api.cohere.ai.