Add OpenAI-compatible chat completions endpoint#16
Conversation
📝 WalkthroughWalkthroughAdds OpenAI chat-completions compatibility: new endpoints ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client as OpenAI Client
participant FastAPI as FastAPI App
participant Provider as Chat Provider
participant Adapter as Content Extractor
participant Converter as Response Converter
Client->>FastAPI: POST /v1/chat/completions (OpenAI request)
FastAPI->>FastAPI: Map OpenAI fields -> internal CompletionRequest
FastAPI->>Provider: _complete_request(CompletionRequest)
Provider-->>FastAPI: Raw completion (string or dict)
FastAPI->>Adapter: extract_openai_content(raw response)
Adapter-->>FastAPI: Extracted message content
FastAPI->>Converter: _to_openai_chat_completion(internal response)
Converter-->>FastAPI: OpenAI-formatted response
FastAPI-->>Client: Return chat.completion payload
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
c663ba9 to
3122a19
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/test_openai_compat.py (1)
21-43: Add coverage for the/v1/chat/completionscompatibility route too.A small parameterized variant over
base_url(/api/and/v1/) would guard both public endpoints against regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_openai_compat.py` around lines 21 - 43, The test test_openai_sdk_chat_completions_uses_compat_route only exercises the /api/ base_url; parameterize it to also test the /v1/ compatibility route by adding a pytest.mark.parametrize over base_url values (e.g., "/api/" and "/v1/") and feeding that into how you construct the OpenAI client (the OpenAI instantiation and sdk_client.chat.completions.create call); ensure the assertion block remains the same so both base_url variants call the same chat_completion dependency/mock and validate response.object, response.model, and response.choices[0] fields for each case.pyproject.toml (1)
24-24: Consider pinning the OpenAI SDK dev dependency to an exact version for deterministic tests.Using
openai>=1,<2allows version drift across minor releases, which can make test results harder to reproduce. Recent 1.x releases have been stable and follow SemVer, but exact pinning (e.g.,openai==1.109.1) is a best practice for test dependencies to ensure consistent behavior across CI runs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` at line 24, The dev dependency spec "openai>=1,<2" in pyproject.toml allows minor-version drift; replace that entry with an exact pinned version (for example change "openai>=1,<2" to "openai==1.109.1") in the same pyproject.toml line, then regenerate/update the lock file (poetry lock / pip-compile / pipenv lock as appropriate) and run the test suite to ensure deterministic CI behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/routes.py`:
- Around line 305-307: The code currently collapses empty-string replies into
None; update the message content assignment so empty strings are preserved and
None is used only for tool-call-only messages: replace the expression
"completion_response.completion or None" used when constructing
OpenAIChatMessage with a conditional that keeps "" as-is and only returns None
when completion_response.completion is exactly None and
completion_response.tool_calls is non-empty (e.g., content =
completion_response.completion if completion_response.completion is not None
else (None if completion_response.tool_calls else "")); target the
OpenAIChatMessage construction where message=... and the symbols
completion_response.completion, completion_response.tool_calls, and
OpenAIChatMessage.
In `@tests/test_openai_compat.py`:
- Around line 44-45: Don't call app.dependency_overrides.clear() in test
cleanup; instead remove only the specific override you set earlier to avoid
affecting other tests. Locate where you set
app.dependency_overrides[<override_key>] (the overridden dependency name used in
this test, e.g., get_openai_client or similar), and replace the clear() call
with targeted removal such as deleting or popping that single key from
app.dependency_overrides so only the test's override is removed.
---
Nitpick comments:
In `@pyproject.toml`:
- Line 24: The dev dependency spec "openai>=1,<2" in pyproject.toml allows
minor-version drift; replace that entry with an exact pinned version (for
example change "openai>=1,<2" to "openai==1.109.1") in the same pyproject.toml
line, then regenerate/update the lock file (poetry lock / pip-compile / pipenv
lock as appropriate) and run the test suite to ensure deterministic CI behavior.
In `@tests/test_openai_compat.py`:
- Around line 21-43: The test test_openai_sdk_chat_completions_uses_compat_route
only exercises the /api/ base_url; parameterize it to also test the /v1/
compatibility route by adding a pytest.mark.parametrize over base_url values
(e.g., "/api/" and "/v1/") and feeding that into how you construct the OpenAI
client (the OpenAI instantiation and sdk_client.chat.completions.create call);
ensure the assertion block remains the same so both base_url variants call the
same chat_completion dependency/mock and validate response.object,
response.model, and response.choices[0] fields for each case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b1159eeb-0bad-48e0-865d-e0e3497d49fe
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
README.mdbackend/adapters.pybackend/routes.pypyproject.tomltests/conftest.pytests/test_openai_compat.py
There was a problem hiding this comment.
♻️ Duplicate comments (1)
backend/routes.py (1)
305-307:⚠️ Potential issue | 🟠 MajorPreserve empty-string content; don’t coerce it to
nullat Line 306.
completion_response.completion or Noneconverts""toNone, which breaks OpenAI chat compatibility for non-tool replies with empty text.Proposed fix
message=OpenAIChatMessage( - content=completion_response.completion or None, + content=( + None + if completion_response.tool_calls + and completion_response.completion == "" + else completion_response.completion + ), tool_calls=completion_response.tool_calls, ),In the OpenAI Chat Completions response schema, should `choices[].message.content` remain `""` for normal assistant messages with empty text, and only be `null` for tool-call-only assistant messages?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/routes.py` around lines 305 - 307, The current code coerces empty assistant text to None by using completion_response.completion or None; instead preserve an empty string for normal assistant replies and only set content to None when the assistant produced no text and is returning tool-only output. Update the OpenAIChatMessage construction (the content field) to assign the raw completion_response.completion (allowing ""), and only set None when completion_response.completion is exactly "" (or falsy) AND completion_response.tool_calls is non-empty (i.e., tool-only responses); leave content as the string otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@backend/routes.py`:
- Around line 305-307: The current code coerces empty assistant text to None by
using completion_response.completion or None; instead preserve an empty string
for normal assistant replies and only set content to None when the assistant
produced no text and is returning tool-only output. Update the OpenAIChatMessage
construction (the content field) to assign the raw
completion_response.completion (allowing ""), and only set None when
completion_response.completion is exactly "" (or falsy) AND
completion_response.tool_calls is non-empty (i.e., tool-only responses); leave
content as the string otherwise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f9155134-0af3-41d1-8784-ce2a55c3e0ba
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
README.mdbackend/adapters.pybackend/routes.pypyproject.tomltests/conftest.pytests/test_openai_compat.py
✅ Files skipped from review due to trivial changes (2)
- README.md
- pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/conftest.py
- tests/test_openai_compat.py
- backend/adapters.py
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/test_openai_compat.py (1)
29-29: Consider adding a guard for empty model list.Accessing
provider_and_models.all_model_names[0]will raiseIndexErrorif no models are available. While unlikely in practice, a skip marker or explicit check would make the failure mode clearer.Proposed improvement
+@pytest.mark.parametrize("base_path", ["/api/", "/v1/"]) +def test_openai_sdk_chat_completions_uses_compat_route(base_path: str) -> None: + """Verify both compatibility routes work with the OpenAI Python SDK.""" + if not provider_and_models.all_model_names: + pytest.skip("No models available for testing") + model = provider_and_models.all_model_names[0]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_openai_compat.py` at line 29, The test directly indexes provider_and_models.all_model_names[0], which will raise IndexError if the list is empty; add a guard before that access (e.g., if not provider_and_models.all_model_names: pytest.skip("no models available for provider") or raise a clear assertion) and then assign model = provider_and_models.all_model_names[0]; reference the symbol provider_and_models.all_model_names and the variable model to locate where to add this check.backend/routes.py (1)
516-539: Consider addingQuery()annotation for theproviderparameter.The
providerparameter works as a query parameter but lacks OpenAPI documentation. Adding an explicitQuery()annotation would provide description metadata for API consumers.Proposed enhancement
+from fastapi import Query + `@router_api.post`("/chat/completions") `@router_v1.post`("/chat/completions") async def post_openai_chat_completion( completion: OpenAIChatCompletionRequest, - provider: str | None = None, + provider: str | None = Query( + None, + description="Provider to use for completion. If not specified, the best available provider will be used.", + ), chat: type[g4f.ChatCompletion] = Depends(chat_completion), ) -> OpenAIChatCompletionResponse:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/routes.py` around lines 516 - 539, The provider query parameter in post_openai_chat_completion is currently unannotated and won't appear in OpenAPI docs; update the function signature to use FastAPI's Query for provider (e.g., provider: str | None = Query(None, description="Selects the completion provider")) so OpenAPI shows it and you can add a helpful description; locate post_openai_chat_completion and replace the plain provider default with Query(...) while leaving completion and chat (Depends(chat_completion)) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/routes.py`:
- Around line 516-539: The provider query parameter in
post_openai_chat_completion is currently unannotated and won't appear in OpenAPI
docs; update the function signature to use FastAPI's Query for provider (e.g.,
provider: str | None = Query(None, description="Selects the completion
provider")) so OpenAPI shows it and you can add a helpful description; locate
post_openai_chat_completion and replace the plain provider default with
Query(...) while leaving completion and chat (Depends(chat_completion))
unchanged.
In `@tests/test_openai_compat.py`:
- Line 29: The test directly indexes provider_and_models.all_model_names[0],
which will raise IndexError if the list is empty; add a guard before that access
(e.g., if not provider_and_models.all_model_names: pytest.skip("no models
available for provider") or raise a clear assertion) and then assign model =
provider_and_models.all_model_names[0]; reference the symbol
provider_and_models.all_model_names and the variable model to locate where to
add this check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 29e463ed-eb49-4bc7-bd2d-2cb7262eb8fd
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
backend/adapters.pybackend/routes.pypyproject.tomltests/conftest.pytests/test_openai_compat.py
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
Summary
This makes the backend usable with OpenAI-compatible clients that append
/chat/completionsto the configured base URL.Changes
POST /api/chat/completionsandPOST /v1/chat/completionsast.literal_evalparsing path with safe structured parsing for concatenated provider payloadsValidation
uv run python -m pytest tests/test_openai_compat.py -quv run python -m pytest tests/test_api.py::test_api_validation -quv run python -m pytest tests/test_chat_completions.py::TestBackwardsCompatibility -quv run python -m pytest tests/test_chat_completions.py::TestToolCalling tests/test_chat_completions.py::TestSupportsToolsField tests/test_chat_completions.py::TestPydanticModels -q