-
Notifications
You must be signed in to change notification settings - Fork 974
chore(wren-ai-service): minor updates #1843
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
Conversation
Caution Review failedThe pull request is closed. WalkthroughThis change removes detailed module-level docstrings from multiple API router modules and updates several request model classes to inherit from Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Router
participant RequestModel
Client->>API_Router: Sends POST/DELETE request
API_Router->>RequestModel: Instantiates BaseRequest-based model
RequestModel-->>API_Router: Validated request (no project_id/configuration fields)
API_Router-->>Client: Processes and responds
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ 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. 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
wren-ai-service/src/web/v1/routers/ask.py
(0 hunks)wren-ai-service/src/web/v1/routers/instructions.py
(2 hunks)wren-ai-service/src/web/v1/routers/question_recommendation.py
(1 hunks)wren-ai-service/src/web/v1/routers/relationship_recommendation.py
(1 hunks)wren-ai-service/src/web/v1/routers/semantics_description.py
(1 hunks)wren-ai-service/src/web/v1/routers/semantics_preparation.py
(0 hunks)wren-ai-service/src/web/v1/routers/sql_answers.py
(0 hunks)wren-ai-service/src/web/v1/routers/sql_corrections.py
(1 hunks)wren-ai-service/src/web/v1/routers/sql_pairs.py
(2 hunks)wren-ai-service/src/web/v1/routers/sql_question.py
(0 hunks)
💤 Files with no reviewable changes (4)
- wren-ai-service/src/web/v1/routers/semantics_preparation.py
- wren-ai-service/src/web/v1/routers/sql_answers.py
- wren-ai-service/src/web/v1/routers/sql_question.py
- wren-ai-service/src/web/v1/routers/ask.py
🧰 Additional context used
🧠 Learnings (6)
wren-ai-service/src/web/v1/routers/relationship_recommendation.py (1)
Learnt from: cyyeh
PR: Canner/WrenAI#1763
File: wren-ai-service/src/pipelines/generation/semantics_description.py:31-33
Timestamp: 2025-06-20T02:37:21.292Z
Learning: In the wren-ai-service codebase, when adding new fields like "alias" to the output of functions that use Pydantic models for validation, the user prefers not to update the corresponding Pydantic model definitions to include these new fields.
wren-ai-service/src/web/v1/routers/sql_corrections.py (2)
Learnt from: cyyeh
PR: Canner/WrenAI#1763
File: wren-ai-service/src/pipelines/generation/semantics_description.py:31-33
Timestamp: 2025-06-20T02:37:21.292Z
Learning: In the wren-ai-service codebase, when adding new fields like "alias" to the output of functions that use Pydantic models for validation, the user prefers not to update the corresponding Pydantic model definitions to include these new fields.
Learnt from: cyyeh
PR: Canner/WrenAI#1112
File: wren-ai-service/src/web/v1/services/sql_question.py:17-28
Timestamp: 2025-01-14T07:45:32.410Z
Learning: In the SqlQuestionRequest model (Python/FastAPI), the query_id field is implemented using a protected _query_id attribute with getter/setter properties, rather than a direct field, as per the team's preference.
wren-ai-service/src/web/v1/routers/instructions.py (3)
Learnt from: onlyjackfrost
PR: Canner/WrenAI#1392
File: wren-ui/src/apollo/server/adaptors/wrenAIAdaptor.ts:477-494
Timestamp: 2025-03-12T09:51:33.688Z
Learning: The instruction deletion API in the WrenAI service handles bulk deletion atomically - either all instructions are deleted or none are. There's no need for handling partial deletion scenarios.
Learnt from: paopa
PR: Canner/WrenAI#1376
File: wren-ai-service/src/pipelines/indexing/instructions.py:100-107
Timestamp: 2025-03-13T03:48:24.664Z
Learning: The `InstructionsCleaner.run()` method in the instructions indexing pipeline only accepts `instruction_ids` and `project_id` parameters, and doesn't support a `delete_all` parameter for unconditional deletion.
Learnt from: onlyjackfrost
PR: Canner/WrenAI#1392
File: wren-ui/src/apollo/server/services/instructionService.ts:107-146
Timestamp: 2025-03-17T09:16:53.878Z
Learning: In the WrenAI project, the instructionResolver merges the instruction ID (from args.where) and project ID (from the current project context) with instruction data before passing to the instructionService, allowing the GraphQL schema to have separate input types while the service layer works with complete objects.
wren-ai-service/src/web/v1/routers/semantics_description.py (1)
Learnt from: cyyeh
PR: Canner/WrenAI#1763
File: wren-ai-service/src/pipelines/generation/semantics_description.py:31-33
Timestamp: 2025-06-20T02:37:21.292Z
Learning: In the wren-ai-service codebase, when adding new fields like "alias" to the output of functions that use Pydantic models for validation, the user prefers not to update the corresponding Pydantic model definitions to include these new fields.
wren-ai-service/src/web/v1/routers/sql_pairs.py (2)
Learnt from: cyyeh
PR: Canner/WrenAI#1763
File: wren-ai-service/src/pipelines/generation/semantics_description.py:31-33
Timestamp: 2025-06-20T02:37:21.292Z
Learning: In the wren-ai-service codebase, when adding new fields like "alias" to the output of functions that use Pydantic models for validation, the user prefers not to update the corresponding Pydantic model definitions to include these new fields.
Learnt from: cyyeh
PR: Canner/WrenAI#1112
File: wren-ai-service/src/web/v1/routers/sql_question.py:59-66
Timestamp: 2025-01-14T07:45:11.981Z
Learning: In the WrenAI codebase, UUID validation for query IDs in API endpoints is not required, as demonstrated in the SQL question router's GET endpoint.
wren-ai-service/src/web/v1/routers/question_recommendation.py (1)
Learnt from: cyyeh
PR: Canner/WrenAI#1763
File: wren-ai-service/src/pipelines/generation/semantics_description.py:31-33
Timestamp: 2025-06-20T02:37:21.292Z
Learning: In the wren-ai-service codebase, when adding new fields like "alias" to the output of functions that use Pydantic models for validation, the user prefers not to update the corresponding Pydantic model definitions to include these new fields.
🧬 Code Graph Analysis (1)
wren-ai-service/src/web/v1/routers/instructions.py (2)
wren-ai-service/src/web/v1/services/__init__.py (1)
BaseRequest
(47-63)wren-ai-service/src/web/v1/services/instructions.py (2)
InstructionsService
(16-176)DeleteRequest
(123-125)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pytest
- GitHub Check: pytest
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (19)
wren-ai-service/src/web/v1/routers/instructions.py (5)
14-14
: LGTM! Import updated correctly for BaseRequest inheritance.The import change properly supports the refactoring to use
BaseRequest
as the base class for request models.
19-21
: LGTM! Proper inheritance from BaseRequest.Inheriting from
BaseRequest
provides access to common fields likeproject_id
,configurations
, andthread_id
while maintaining the specificinstructions
field. This standardizes the request model and eliminates code duplication.
50-52
: LGTM! Consistent inheritance from BaseRequest.The
DeleteRequest
class correctly inherits fromBaseRequest
, providing access to common fields likeproject_id
which is essential for properly scoping instruction deletions. This maintains consistency with thePostRequest
refactoring.
38-40
: Endpoint logic works correctly with BaseRequest inheritance.The use of
**request.model_dump()
properly includes all fields from theBaseRequest
base class when creating theInstructionsService.IndexRequest
. This ensures fields likeproject_id
andconfigurations
are passed to the service layer.
65-68
: Delete endpoint logic maintains consistency.The delete endpoint correctly uses the same
**request.model_dump()
pattern to include allBaseRequest
fields when creating the service request. This ensures proper project scoping for instruction deletions.wren-ai-service/src/web/v1/routers/sql_corrections.py (3)
14-14
: LGTM: Import statement properly updated for BaseRequest.The addition of
BaseRequest
to the import statement aligns with the class inheritance change below.
19-19
: LGTM: Class inheritance change aligns with codebase refactoring.The change from
BaseModel
toBaseRequest
is consistent with the broader refactoring effort mentioned in the summary across multiple router modules. This standardization improves consistency across the API.
42-44
: No action needed: BaseRequest still provides model_dump()BaseRequest inherits from Pydantic’s BaseModel (see wren-ai-service/src/web/v1/services/init.py), so
model_dump()
remains available on all subclasses, includingPostRequest
.wren-ai-service/src/web/v1/routers/question_recommendation.py (4)
14-14
: LGTM! Import updated to support class inheritance change.The import change from
Configuration
toBaseRequest
aligns with the updatedPostRequest
class inheritance and removes unused imports.
21-21
: Good field improvements for request flexibility.The default empty list for
previous_questions
and the newallow_data_preview
boolean field with defaultTrue
improve the API's usability by making these parameters optional with sensible defaults.Also applies to: 25-25
18-18
: Documentation cleanup aligns with broader standardization effort.The removal of the module-level docstring is consistent with the broader documentation standardization across router modules mentioned in the AI summary.
19-19
: Confirm BaseRequest Handles All Required FieldsVerified that
BaseRequest
(inwren-ai-service/src/web/v1/services/__init__.py
) already declares and manages the following, covering the previously removed fields:
project_id: Optional[str]
configurations: Configuration
(alias supports bothconfiguration
andconfigurations
)thread_id: Optional[str]
_query_id
with correspondingquery_id
propertyNo further action required—this refactoring correctly centralizes common request parameters. Changes approved.
wren-ai-service/src/web/v1/routers/semantics_description.py (2)
14-14
: LGTM! Import standardization aligns with refactoring effort.The change from importing
Configuration
toBaseRequest
is consistent with the broader refactoring effort to standardize request models across router modules.
19-19
: BaseRequest already includesproject_id
andconfigurations
.The
BaseRequest
class insrc/web/v1/services/__init__.py
defines bothproject_id: Optional[str]
andconfigurations: Configuration
, so yourPostRequest(BaseRequest)
inherits the necessary fields. No further changes needed.wren-ai-service/src/web/v1/routers/sql_pairs.py (3)
15-15
: Import change supports the refactoring correctly.The addition of
BaseRequest
to the import statement is appropriate and necessary for the inheritance changes in the request models.
49-50
: Inheritance change is consistent with PostRequest.The change from
BaseModel
toBaseRequest
maintains consistency with thePostRequest
class and supports the codebase standardization effort.
20-21
: BaseRequest correctly inherits from Pydantic BaseModelVerified that
BaseRequest
is defined asclass BaseRequest(BaseModel)
and includes all required fields and methods (e.g.,model_dump
), soPostRequest(BaseRequest)
retains full Pydantic validation/serialization. No further changes needed.wren-ai-service/src/web/v1/routers/relationship_recommendation.py (2)
14-14
: Import change looks good for the refactoring.The switch from importing
Configuration
toBaseRequest
aligns with the standardization effort across router modules.
44-46
: Verify field access matches the updated request model.The endpoint is accessing
request.project_id
andrequest.configuration
, but these fields were removed fromPostRequest
. IfBaseRequest
provides these fields, this should work correctly, but it needs verification.
Summary by CodeRabbit
Refactor
Documentation