-
Notifications
You must be signed in to change notification settings - Fork 2
fix incorrect openai-compatible endpoint paths #7
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes update the handling of API endpoint paths for OpenAI-compatible interfaces. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ExampleScript
participant AI
participant OpenAICompatibleProvider
participant GeminiAPI
User->>ExampleScript: Run openai_compatible_example.exs
ExampleScript->>AI: generate_text(prompt, model)
AI->>OpenAICompatibleProvider: do_generate/2 with base_url (may include /v1 or /v1beta)
OpenAICompatibleProvider->>GeminiAPI: POST /chat/completions (no hardcoded /v1 in path)
GeminiAPI-->>OpenAICompatibleProvider: Response
OpenAICompatibleProvider-->>AI: Generated text
AI-->>ExampleScript: Generated text
ExampleScript-->>User: Output result
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
✨ 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 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: 3
♻️ Duplicate comments (1)
lib/ai/providers/openai_compatible/chat_language_model.ex (1)
246-248: Duplicate hard-coded endpoint & same double-slash risk
do_stream/2re-implements the exact URL assembly done inmake_api_request/3, including the potential “//” issue. Consider centralising this logic in one helper to stay DRY and fix the bug once.
🧹 Nitpick comments (7)
lib/ai.ex (1)
223-224: Inconsistent/v1handling between chat vs. completion helpers
openai/2now hard-codes/v1into the defaultbase_url, whileopenai_completion/2still expects the version segment to be supplied in the path inside its provider module. This asymmetry is surprising and easy to miss when switching between the two helpers. Please either:
- Align the defaults (both include
/v1), or- Add explicit docs/comments explaining why the two helpers differ.
A small clarification now will save others head-scratching later.
test/ai/providers/openai_compatible/generate_text_test.exs (1)
12-15: Nice update — consider an extra case with a trailing “/”The new
/v1base URL is covered, but adding one test withbase_url: "https://api.example.com/v1/"would exercise the double-slash edge case flagged in the implementation.test/ai/openai_compatible_test.exs (1)
33-35: Add coverage for trailing-slash base URLsAs with the provider tests, adding one variant that passes
"https://api.example.com/v1/"would protect against the double-slash bug.Also applies to: 44-45
examples/openai_compatible_example.exs (4)
11-16: Prefer System.fetch_env/1 for env var handlingfetch_env avoids nil checks and expresses intent clearly.
- api_key = System.get_env("GOOGLE_API_KEY") - - if is_nil(api_key) do - IO.puts("Error: GOOGLE_API_KEY environment variable not set") - System.halt(1) - end + api_key = + case System.fetch_env("GOOGLE_API_KEY") do + {:ok, key} -> + key + + :error -> + IO.puts("Error: GOOGLE_API_KEY environment variable not set") + System.halt(1) + end
9-9: Nit: IO.puts already appends a newlineRemove the explicit \n to avoid a double blank line.
- IO.puts("Starting generate_text example...\n") + IO.puts("Starting generate_text example...")
1-3: Nit: Capitalization/wording in header commentsTweak capitalization and trailing space.
-# Sample script demonstrating AI.generate_text usage with an openai-compatible api -# Run from the elixir-ai-sdk root directory with: +# Sample script demonstrating AI.generate_text usage with an OpenAI-compatible API +# Run from the elixir-ai-sdk root directory with:
5-6: Add a clarifying note about base_url expectationsSince this example exists to illustrate the /v1 path change, a brief note helps users avoid common misconfigurations.
# Make sure you've set the GOOGLE_API_KEY environment variable +# +# Note: +# - For OpenAI, base_url typically includes /v1 (e.g., https://api.openai.com/v1). +# - For Gemini's OpenAI-compatible API, do NOT add /v1; use v1beta/openai as shown below.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/openai_compatible_example.exs(1 hunks)lib/ai.ex(3 hunks)lib/ai/providers/openai_compatible/chat_language_model.ex(2 hunks)test/ai/openai_compatible_test.exs(6 hunks)test/ai/openai_test.exs(1 hunks)test/ai/providers/openai_compatible/generate_text_test.exs(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
test/ai/openai_test.exs (1)
lib/ai.ex (1)
openai(215-251)
examples/openai_compatible_example.exs (1)
lib/ai.ex (1)
openai_compatible(178-187)
test/ai/openai_compatible_test.exs (1)
lib/ai.ex (1)
openai_compatible(178-187)
🔇 Additional comments (5)
test/ai/openai_test.exs (1)
76-78: LGTM – test keeps parity with helper changetest/ai/openai_compatible_test.exs (2)
18-29: Base URL updates look correctTests now reflect the
/v1shift and still assert URLs accurately.
93-97: Consistency achieved – good jobAlso applies to: 166-168, 232-234
examples/openai_compatible_example.exs (2)
24-25: LGTM: Correct base_url for Gemini’s OpenAI-compatible endpointUsing https://generativelanguage.googleapis.com/v1beta/openai (without an extra /v1) aligns with the PR objective and prevents 404s.
33-34: LGTM: Running the example at script load is appropriate hereAuto-executing the example from an .exs script matches the README-style usage.
| {:ok, result} = | ||
| AI.generate_text(%{ | ||
| prompt: "is elixir a statically typed programming language?" | ||
| model: | ||
| AI.openai_compatible( | ||
| "gemini-2.0-flash", | ||
| base_url: "https://generativelanguage.googleapis.com/v1beta/openai", | ||
| api_key: api_key | ||
| ), | ||
| }) | ||
|
|
||
| IO.puts(result.text) |
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.
🛠️ Refactor suggestion
Make the example robust: handle {:error, reason} and avoid match crashes (also fixes the missing comma)
Pattern matching on {:ok, result} will crash on errors. Wrap the call in a case to print failures and exit with non‑zero status. This diff also includes the missing comma fix.
- {:ok, result} =
- AI.generate_text(%{
- prompt: "is elixir a statically typed programming language?"
- model:
- AI.openai_compatible(
- "gemini-2.0-flash",
- base_url: "https://generativelanguage.googleapis.com/v1beta/openai",
- api_key: api_key
- ),
- })
-
- IO.puts(result.text)
+ case AI.generate_text(%{
+ prompt: "is elixir a statically typed programming language?",
+ model:
+ AI.openai_compatible(
+ "gemini-2.0-flash",
+ base_url: "https://generativelanguage.googleapis.com/v1beta/openai",
+ api_key: api_key
+ )
+ }) do
+ {:ok, result} ->
+ IO.puts(result.text)
+
+ {:error, reason} ->
+ IO.puts("generate_text error: #{inspect(reason)}")
+ System.halt(1)
+ end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {:ok, result} = | |
| AI.generate_text(%{ | |
| prompt: "is elixir a statically typed programming language?" | |
| model: | |
| AI.openai_compatible( | |
| "gemini-2.0-flash", | |
| base_url: "https://generativelanguage.googleapis.com/v1beta/openai", | |
| api_key: api_key | |
| ), | |
| }) | |
| IO.puts(result.text) | |
| case AI.generate_text(%{ | |
| prompt: "is elixir a statically typed programming language?", | |
| model: | |
| AI.openai_compatible( | |
| "gemini-2.0-flash", | |
| base_url: "https://generativelanguage.googleapis.com/v1beta/openai", | |
| api_key: api_key | |
| ) | |
| }) do | |
| {:ok, result} -> | |
| IO.puts(result.text) | |
| {:error, reason} -> | |
| IO.puts("generate_text error: #{inspect(reason)}") | |
| System.halt(1) | |
| end |
🤖 Prompt for AI Agents
In examples/openai_compatible_example.exs around lines 18 to 29, the code
directly pattern matches on {:ok, result}, which will crash if an error tuple is
returned. To fix this, replace the direct match with a case statement that
handles both {:ok, result} and {:error, reason} cases, printing the error reason
and exiting with a non-zero status on failure. Also, add the missing comma after
the prompt string to correct the syntax.
| case make_api_request(model.provider, "/chat/completions", request_body) do | ||
| {:ok, response} -> |
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.
🛠️ Refactor suggestion
Possible “//” in URL when base_url ends with a slash
make_api_request/3 blindly concatenates:
url = "#{provider.base_url}#{path}"If a caller supplies base_url: "https://api.example.com/v1/", the final URL becomes
https://api.example.com/v1//chat/completions, which many servers treat differently or reject.
Guard against this by trimming one side or, better, using Path.join/2-style logic:
-url = "#{provider.base_url}#{path}"
+url = "#{String.trim_trailing(provider.base_url, "/")}#{path}"You might also extract "/chat/completions" into a module attribute to avoid the duplication seen here and at Line 247.
🤖 Prompt for AI Agents
In lib/ai/providers/openai_compatible/chat_language_model.ex around lines 72 to
73, the URL construction in make_api_request/3 concatenates base_url and path
directly, which can cause double slashes if base_url ends with a slash. Fix this
by trimming the trailing slash from base_url or the leading slash from path
before concatenation, or implement Path.join/2-style logic to join them safely.
Additionally, extract the "/chat/completions" string into a module attribute to
avoid duplication and improve maintainability.
fixes #6
Summary by CodeRabbit
New Features
Bug Fixes
Tests