-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make /v1 endpoint optional in order to support Ollama models #2181
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe pull request introduces a modification to the URL validation regex pattern in the OpenAI settings component. The change allows the base URL to optionally include Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/client/src/pages/dashboard/settings/_sections/openai.tsx (1)
30-30
: LGTM! The regex change successfully restores Ollama compatibility.The updated regex pattern now correctly allows URLs with an optional
/v1
endpoint, fixing the regression for Ollama model support.However, consider making the pattern more flexible:
- .regex(/^https?:\/\/[^/]+\/?(v1)?$/, "That doesn't look like a valid URL") + .regex(/^https?:\/\/[^/]+(:\d+)?\/?(v1)?$/, "That doesn't look like a valid URL")This improvement adds support for port numbers, which is essential for local Ollama instances (e.g.,
localhost:11434
).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/client/src/pages/dashboard/settings/_sections/openai.tsx
(1 hunks)
🔇 Additional comments (2)
apps/client/src/pages/dashboard/settings/_sections/openai.tsx (2)
Line range hint
91-102
: LGTM! Clear and accurate documentation.The component provides excellent documentation with:
- Clear instructions for both OpenAI and Ollama setup
- Accurate example URL format
- Proper security and privacy notices
30-30
: Verify URL handling across the codebase.Let's ensure consistent URL handling throughout the codebase:
✅ Verification successful
URL validation is correctly isolated to OpenAI settings
The OpenAI URL validation is specific to the settings section and doesn't need replication elsewhere, as all API interactions are properly centralized in dedicated service files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for URL validation patterns and API endpoint handling echo "Searching for URL validation patterns..." rg -l 'https?:\/\/[^/]+' --type ts --type tsx echo -e "\nSearching for OpenAI/Ollama API calls..." rg -l 'baseURL.*openai|openai.*baseURL' --type ts --type tsx echo -e "\nSearching for v1 endpoint usage..." rg -l '\/v1\/' --type ts --type tsxLength of output: 612
Script:
#!/bin/bash # Description: Search for URL validation patterns and API endpoint handling with corrected regex # Add tsx as a recognized file type rg --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' echo "Searching for URL validation patterns..." rg -l 'https?://[^/]+' --type ts --type tsx echo -e "\nSearching for OpenAI/Ollama API calls..." rg -l 'baseURL.*openai|openai.*baseURL' --type ts --type tsx echo -e "\nSearching for v1 endpoint usage..." rg -l '/v1/' --type ts --type tsx # Also search for specific validation patterns echo -e "\nSearching for URL validation code..." rg -l 'regex.*url|validate.*url|isValidUrl' --type ts --type tsxLength of output: 1129
Script:
#!/bin/bash # Description: Search for URL validation and API endpoints with simplified approach echo "Searching for URL validation patterns..." fd -e ts -e tsx | xargs rg -l 'https?://[^/]+' echo -e "\nSearching for OpenAI API related code..." fd -e ts -e tsx | xargs rg -l 'openai' echo -e "\nSearching for v1 endpoint usage..." fd -e ts -e tsx | xargs rg -l '/v1' echo -e "\nSearching for URL validation code..." fd -e ts -e tsx | xargs rg -l 'regex.*url|validate.*url|isValidUrl'Length of output: 2927
@vvok12 Can we update the PR to just check if the URL starts with https or http? So the url path or other routes just shouldn't matter. This might be useful when v1 changes to v2 someday and we can make use of the app even then. |
Earlier commit removes the ability to specify /v1 which breaks Ollama integration.
This PR fixes regression by making /v1 ending optional
Summary by CodeRabbit
/v1
endpoint segment