Open
Conversation
Contributor
Reviewer's GuideThis PR implements a fully functional /chat endpoint backed by the OpenRouterService, including request decoding, history validation, token retrieval from the database, error mapping, and response serialization; it also persists user API keys, refactors the application factory for dependency injection and error handling, defines a reusable OpenRouter service abstraction, adds OpenAPI documentation, and expands both unit and integration tests with in-memory DB fixtures and HTTPX mocks. Sequence Diagram for /chat Endpoint ProcessingsequenceDiagram
actor User
participant CR as ChatResource
participant DB as Database
participant Helper as "chat_with_service()"
participant ORS as OpenRouterService
participant ExtAPI as "OpenRouter External API"
User->>CR: POST /chat (message, history, model)
CR->>CR: Decode request & validate history
CR->>DB: SELECT openrouter_token_enc FROM UserAccount
DB-->>CR: User's token (or null)
alt Token Found
CR->>Helper: chat_with_service(token, history, model)
Helper->>ORS: chat_completion(token, history, model)
ORS->>ExtAPI: Request completion
ExtAPI-->>ORS: Completion response / Error
alt Successful Completion
ORS-->>Helper: Completion
Helper-->>CR: Completion
CR->>CR: answer = completion.choices[0].message.content
CR-->>User: HTTP 200 OK {"answer": ...}
else OpenRouter Timeout from ORS
ORS-->>Helper: OpenRouterTimeoutError (from OpenRouterAsyncClient)
Helper-->>CR: OpenRouterServiceTimeoutError
CR-->>User: HTTP 504 Gateway Timeout
else OpenRouter Other Error from ORS
ORS-->>Helper: OpenRouterError (e.g. NetworkError)
Helper-->>CR: OpenRouterServiceBadGatewayError
CR-->>User: HTTP 502 Bad Gateway
end
else Token Not Found
CR-->>User: HTTP 400 Bad Request (missing OpenRouter token)
end
alt Invalid Request (JSON, message field, history item)
User->>CR: POST /chat (invalid data)
CR->>CR: Fail validation
CR-->>User: HTTP 400 Bad Request
end
Sequence Diagram for /auth/openrouter-token EndpointsequenceDiagram
actor User
participant OTR as OpenRouterTokenResource
participant DB as Database
User->>OTR: POST /auth/openrouter-token (api_key)
OTR->>OTR: Validate api_key
alt Valid api_key
OTR->>DB: UPDATE UserAccount SET openrouter_token_enc = api_key.encode()
DB-->>OTR: Success
OTR-->>User: HTTP 204 No Content
else Invalid api_key (not string)
OTR-->>User: HTTP 400 Bad Request (`api_key` field required)
end
Entity Relationship Diagram for UserAccount ChangeserDiagram
UserAccount {
string google_sub PK "User's Google Subject ID"
bytes openrouter_token_enc "Encrypted OpenRouter Token (updated)"
}
Class Diagram for API Resource and Request ClassesclassDiagram
class ChatRequest {
<<msgspec.Struct>>
+message: str
+history: list~dict~ | None
+model: str | None
}
class ChatResource {
- _service: OpenRouterService
- _session_factory: Callable_AsyncSession
+ __init__(service: OpenRouterService, session_factory: Callable_AsyncSession)
+ on_post(req: Request, resp: Response) void
}
class OpenRouterTokenResource {
- _session_factory: Callable_AsyncSession
+ __init__(session_factory: Callable_AsyncSession)
+ on_post(req: Request, resp: Response) void
}
ChatResource ..> ChatRequest : decodes
ChatResource o-- OpenRouterService : uses
ChatResource o-- Callable_AsyncSession : uses
OpenRouterTokenResource o-- Callable_AsyncSession : uses
class OpenRouterService
class Callable_AsyncSession {
<<TypeAlias>>
Callable[[], AsyncSession]
}
class Request
class Response
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Consider returning HTTP 500 instead of HTTP 400 when the OpenRouter API key is missing, since that’s a server configuration issue rather than a bad client request.
- Extract the OpenRouter client and configuration (API key, default model) into a separate service or dependency to improve testability and decouple environment lookups from the request handler.
- Add validation or a whitelist for the incoming
modelparameter to ensure only supported model identifiers are used and prevent invalid requests.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Owner
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- You’re manually decoding and validating the request body in ChatResource.on_post; consider using the ChatRequest msgspec.Struct or a shared validator to centralize schema validation and reduce boilerplate.
- The db_session_factory fixture is duplicated in both integration and unit test files—moving it into a shared conftest.py will DRY up the test setup.
- Reading the raw body via req.bounded_stream.read() bypasses Falcon’s built-in request parsing and limits—consider integrating a custom media handler or using req.get_media() with msgspec to enforce size and content-type constraints.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| except OpenRouterServiceBadGatewayError as exc: | ||
| raise falcon.HTTPBadGateway(description=str(exc)) from None # pyright: ignore[reportUnknownArgumentType] | ||
|
|
||
| answer = completion.choices[0].message.content or "" |
Contributor
There was a problem hiding this comment.
suggestion: Handle case when completion.choices is empty
Add a check to ensure choices is not empty before accessing choices[0] to prevent IndexError.
|
|
||
| @pytest.mark.asyncio | ||
| async def test_store_token_not_implemented(app: asgi.App) -> None: | ||
| async def test_store_token( |
Contributor
There was a problem hiding this comment.
suggestion (testing): Test /auth/openrouter-token with non-string api_key.
Please add a test where api_key is present but not a string (e.g., integer, boolean, or null), and verify that it returns an HTTPBadRequest with the appropriate error message.
leynos
force-pushed
the
codex/implement-/chat-endpoint-with-deepseek-model
branch
from
d55871f to
c0b0926
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
ruff check src/bournemouth/resources.py tests/test_resources.pypyrightpytest -qhttps://chatgpt.com/codex/tasks/task_e_6844d08408f083228c290ec6dace05b7
Summary by Sourcery
Implement full OpenRouter-backed chat functionality and API key storage with proper DB integration, service abstraction, error handling, OpenAPI docs, and updated tests.
New Features:
Enhancements:
Documentation:
Tests: