feat: Claude AI configuration supports custom base URL #2249
Conversation
|
|
|
Caution Review failedThe pull request is closed. WalkthroughReplaces a hard-coded Anthropik/Anthropic messages endpoint with a computed endpoint taken from request.Opts.BaseURL when provided (defaults to https://api.anthropic.com/v1/messages after trimming). The anthropic-version header is now set from request.Opts.APIVersion if non-empty (default remains 2023-06-01). In the Google backend, a stray trailing whitespace line after appending the API key was removed and cancellation handling in the streaming loop was changed from a non-blocking select to a direct ctx.Err() check that sends an AI error and breaks the loop. No public signatures were changed. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/waveai/anthropicbackend.go (1)
191-191: Avoid extra allocation when building the request bodyUsing strings.NewReader(string(reqBody)) causes an unnecessary []byte→string allocation. Use bytes.NewReader(reqBody) instead.
Apply this diff:
- req, err := http.NewRequestWithContext(ctx, "POST", endpoint, strings.NewReader(string(reqBody))) + req, err := http.NewRequestWithContext(ctx, "POST", endpoint, bytes.NewReader(reqBody))Additional change outside the selected range: ensure "bytes" is imported.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/waveai/anthropicbackend.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
pkg/waveai/anthropicbackend.go (1)
pkg/wcloud/wcloud.go (1)
APIVersion(34-34)
🔇 Additional comments (1)
pkg/waveai/anthropicbackend.go (1)
200-205: LGTM: configurable anthropic-version with sensible defaultThis correctly defaults to "2023-06-01" and allows overrides via request.Opts.APIVersion while keeping the header explicit.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
pkg/waveai/anthropicbackend.go (1)
228-229: Avoid extra allocations: use bytes.NewReader for JSON bodyConverting []byte to string and back to an io.Reader causes an unnecessary allocation. Prefer bytes.NewReader.
Apply this diff:
- req, err := http.NewRequestWithContext(ctx, "POST", endpoint, strings.NewReader(string(reqBody))) + req, err := http.NewRequestWithContext(ctx, "POST", endpoint, bytes.NewReader(reqBody))Additional change required outside this range:
import ( + "bytes" "bufio" "context"
🧹 Nitpick comments (1)
pkg/waveai/anthropicbackend.go (1)
34-43: Optionally support scheme-less inputs by defaulting to httpsSome users paste hosts without a scheme (e.g., proxy.internal:8443). You can reduce friction by defaulting such inputs to https before enforcing the scheme check.
Apply this diff:
parsedURL, err := url.Parse(baseURL) if err != nil { return "", fmt.Errorf("failed to parse base URL: %v", err) } - // Validate scheme + // Allow scheme-less inputs like "proxy.internal:8443" by defaulting to https + if parsedURL.Scheme == "" && parsedURL.Host == "" { + if attempt, perr := url.Parse("https://" + baseURL); perr == nil { + parsedURL = attempt + } + } + + // Validate scheme if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" { return "", errors.New("base URL must use http or https scheme") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/waveai/anthropicbackend.go(4 hunks)
🔇 Additional comments (2)
pkg/waveai/anthropicbackend.go (2)
217-227: Endpoint override flow looks goodClean fallback to the default messages endpoint and clear error propagation when the custom base URL is invalid. Nice separation via buildAnthropicEndpoint.
237-241: Configurable anthropic-version header is correctly implementedSane default retained and override supported via request.Opts.APIVersion. Header key is correct.
pkg/waveai/anthropicbackend.go
Outdated
| // buildAnthropicEndpoint safely constructs the Anthropic API endpoint from a base URL | ||
| // It validates the scheme, preserves query parameters and host, and handles common path patterns | ||
| func buildAnthropicEndpoint(baseURL string) (string, error) { | ||
| if baseURL == "" { | ||
| return "", errors.New("base URL cannot be empty") | ||
| } | ||
|
|
||
| // Parse the base URL | ||
| parsedURL, err := url.Parse(baseURL) | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to parse base URL: %v", err) | ||
| } | ||
|
|
||
| // Validate scheme | ||
| if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" { | ||
| return "", errors.New("base URL must use http or https scheme") | ||
| } | ||
|
|
||
| // Handle different path patterns: | ||
| // - https://custom -> https://custom/v1/messages | ||
| // - https://custom/ -> https://custom/v1/messages | ||
| // - https://custom/v1 -> https://custom/v1/messages | ||
| // - https://custom/v1/ -> https://custom/v1/messages | ||
| // - https://custom/v1/messages -> https://custom/v1/messages | ||
| // - https://custom/v1/messages/ -> https://custom/v1/messages | ||
|
|
||
| currentPath := strings.TrimRight(parsedURL.Path, "/") | ||
|
|
||
| var targetPath string | ||
| if strings.HasSuffix(currentPath, "/messages") { | ||
| targetPath = currentPath | ||
| } else if strings.HasSuffix(currentPath, "/v1") { | ||
| targetPath = currentPath + "/messages" | ||
| } else { | ||
| // Empty path or any other path gets /v1/messages appended | ||
| targetPath = path.Join(currentPath, "/v1/messages") | ||
| } | ||
|
|
||
| // Construct the final URL preserving query parameters | ||
| parsedURL.Path = targetPath | ||
| return parsedURL.String(), nil | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add unit tests for buildAnthropicEndpoint to lock in behavior
Given the URL-joining edge cases, unit tests will prevent regressions and document expected inputs/outputs (including path preservation, trailing slashes, and queries).
Suggested test cases:
- https://custom → https://custom/v1/messages
- https://custom/ → https://custom/v1/messages
- https://custom/v1 → https://custom/v1/messages
- https://custom/v1/ → https://custom/v1/messages
- https://custom/v1/messages → https://custom/v1/messages
- https://custom/v1/messages/ → https://custom/v1/messages
- https://custom/api → https://custom/api/v1/messages
- https://custom/api/ → https://custom/api/v1/messages
- https://custom/api?x=1 → https://custom/api/v1/messages?x=1
- proxy.internal:8443 (no scheme) → https://proxy.internal:8443/v1/messages (if you accept the optional refactor)
If you’d like, I can draft pkg/waveai/anthropicbackend_test.go covering these cases.
|
thanks for submitting this! looks pretty straightforward, will take a look and try to get this merged. |
Because I am on an internal network, I need to use a proxy server to access Claude's services. However, Wave currently does not provide the ability to configure the base URL, so I have added this feature in hopes of being able to use it.