chore(wren-ai-service): update apis #1726
Conversation
cyyeh
added
module/ai-service
WalkthroughThis change introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Service
participant BaseRequest
participant Event/Resource
participant Cache
Client->>API_Service: Send request (with request_from)
API_Service->>BaseRequest: Parse request (inherits request_from)
API_Service->>Event/Resource: Create/update with request_from
API_Service->>Cache: Store event/resource with request_from
API_Service-->>Client: Respond (metadata includes request_from)
Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
wren-ai-service/src/web/v1/services/__init__.py(3 hunks)wren-ai-service/src/web/v1/services/ask.py(5 hunks)wren-ai-service/src/web/v1/services/chart.py(2 hunks)wren-ai-service/src/web/v1/services/chart_adjustment.py(3 hunks)wren-ai-service/src/web/v1/services/instructions.py(5 hunks)wren-ai-service/src/web/v1/services/question_recommendation.py(7 hunks)wren-ai-service/src/web/v1/services/relationship_recommendation.py(5 hunks)wren-ai-service/src/web/v1/services/semantics_description.py(4 hunks)wren-ai-service/src/web/v1/services/semantics_preparation.py(3 hunks)wren-ai-service/src/web/v1/services/sql_answer.py(2 hunks)wren-ai-service/src/web/v1/services/sql_corrections.py(5 hunks)wren-ai-service/src/web/v1/services/sql_pairs.py(5 hunks)wren-ai-service/src/web/v1/services/sql_question.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
wren-ai-service/src/web/v1/services/semantics_preparation.py (1)
wren-ai-service/src/web/v1/services/__init__.py (1)
BaseRequest(47-60)
wren-ai-service/src/web/v1/services/sql_question.py (1)
wren-ai-service/src/web/v1/services/__init__.py (1)
BaseRequest(47-60)
wren-ai-service/src/web/v1/services/sql_answer.py (1)
wren-ai-service/src/web/v1/services/__init__.py (2)
BaseRequest(47-60)SSEEvent(33-43)
wren-ai-service/src/web/v1/services/semantics_description.py (3)
wren-ai-service/src/web/v1/services/__init__.py (2)
BaseRequest(47-60)MetadataTraceable(7-21)wren-ai-service/src/web/v1/services/relationship_recommendation.py (3)
Resource(21-31)Error(22-24)_handle_exception(44-59)wren-ai-service/src/web/v1/services/instructions.py (2)
Error(24-26)_handle_exception(45-60)
wren-ai-service/src/web/v1/services/chart_adjustment.py (1)
wren-ai-service/src/web/v1/services/__init__.py (3)
BaseRequest(47-60)query_id(55-56)query_id(59-60)
wren-ai-service/src/web/v1/services/instructions.py (5)
wren-ai-service/src/web/v1/services/__init__.py (3)
BaseRequest(47-60)MetadataTraceable(7-21)with_metadata(8-14)wren-ai-service/src/web/v1/services/question_recommendation.py (3)
Event(22-28)Error(18-20)_handle_exception(43-58)wren-ai-service/src/web/v1/services/relationship_recommendation.py (2)
Error(22-24)_handle_exception(44-59)wren-ai-service/src/web/v1/services/semantics_description.py (2)
Error(19-21)_handle_exception(39-54)wren-ai-service/src/web/v1/routers/instructions.py (1)
DeleteRequest(111-113)
wren-ai-service/src/web/v1/services/sql_pairs.py (2)
wren-ai-service/src/web/v1/services/__init__.py (3)
BaseRequest(47-60)MetadataTraceable(7-21)with_metadata(8-14)wren-ai-service/src/web/v1/routers/sql_pairs.py (1)
DeleteRequest(112-114)
🪛 Pylint (3.3.7)
wren-ai-service/src/web/v1/services/semantics_preparation.py
[refactor] 17-17: Too few public methods (1/2)
(R0903)
wren-ai-service/src/web/v1/services/sql_question.py
[refactor] 17-17: Too few public methods (1/2)
(R0903)
wren-ai-service/src/web/v1/services/chart.py
[refactor] 16-16: Too few public methods (1/2)
(R0903)
[refactor] 23-23: Too few public methods (0/2)
(R0903)
[refactor] 28-28: Too few public methods (1/2)
(R0903)
wren-ai-service/src/web/v1/services/sql_answer.py
[refactor] 17-17: Too few public methods (1/2)
(R0903)
wren-ai-service/src/web/v1/services/relationship_recommendation.py
[refactor] 17-17: Too few public methods (1/2)
(R0903)
wren-ai-service/src/web/v1/services/semantics_description.py
[refactor] 56-56: Too few public methods (1/2)
(R0903)
wren-ai-service/src/web/v1/services/chart_adjustment.py
[refactor] 27-27: Too few public methods (1/2)
(R0903)
[refactor] 34-34: Too few public methods (0/2)
(R0903)
[refactor] 39-39: Too few public methods (1/2)
(R0903)
wren-ai-service/src/web/v1/services/__init__.py
[error] 20-20: Instance of 'MetadataTraceable' has no 'request_from' member
(E1101)
[refactor] 47-47: Too few public methods (1/2)
(R0903)
wren-ai-service/src/web/v1/services/instructions.py
[refactor] 62-62: Too few public methods (1/2)
(R0903)
[refactor] 123-123: Too few public methods (1/2)
(R0903)
wren-ai-service/src/web/v1/services/sql_corrections.py
[refactor] 57-57: Too few public methods (1/2)
(R0903)
wren-ai-service/src/web/v1/services/ask.py
[refactor] 22-22: Too few public methods (1/2)
(R0903)
[refactor] 32-32: Too few public methods (0/2)
(R0903)
[refactor] 37-37: Too few public methods (1/2)
(R0903)
[refactor] 95-95: Too few public methods (1/2)
(R0903)
[refactor] 102-102: Too few public methods (0/2)
(R0903)
[refactor] 107-107: Too few public methods (1/2)
(R0903)
wren-ai-service/src/web/v1/services/question_recommendation.py
[refactor] 155-155: Too few public methods (1/2)
(R0903)
wren-ai-service/src/web/v1/services/sql_pairs.py
[refactor] 54-54: Too few public methods (1/2)
(R0903)
[refactor] 97-97: Too few public methods (1/2)
(R0903)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pytest
- GitHub Check: pytest
- GitHub Check: Analyze (go)
🔇 Additional comments (50)
wren-ai-service/src/web/v1/services/chart.py (3)
10-10: Import addition looks good.The BaseRequest import is correctly added to support the inheritance changes.
16-16: Inheritance refactoring is well-implemented.Both ChartRequest and StopChartRequest now correctly inherit from BaseRequest, which centralizes common fields like
project_id,thread_id,configurations, andrequest_from. This reduces code duplication and follows the DRY principle.Also applies to: 28-28
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 16-16: Too few public methods (1/2)
(R0903)
94-94: Request tracking metadata correctly added.The
request_fromfield is properly added to the metadata to track whether the request originated from "ui" or "api", which aligns with the broader standardization effort.wren-ai-service/src/web/v1/services/semantics_preparation.py (4)
11-11: Import addition is correct.The BaseRequest import is properly added to support the class inheritance changes.
17-17: Inheritance change follows standardization pattern.SemanticsPreparationRequest now inherits from BaseRequest, which provides common fields and reduces code duplication. The removed
project_idfield is now inherited from the base class.🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 17-17: Too few public methods (1/2)
(R0903)
69-69: Request source tracking added consistently.The
request_fromfield is correctly added to the metadata, maintaining consistency with the tracking pattern established across other services.
94-110: Code formatting improvement.The cache assignment has been reformatted for better readability while maintaining the same logic and functionality.
wren-ai-service/src/web/v1/services/sql_answer.py (3)
11-11: Imports correctly updated.Both BaseRequest and SSEEvent imports are properly added to support the class inheritance and SSE functionality.
17-17: Inheritance change aligns with standardization.SqlAnswerRequest now inherits from BaseRequest, removing redundant field declarations and leveraging the centralized common fields like
project_id,configurations, andrequest_from.🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 17-17: Too few public methods (1/2)
(R0903)
68-69: Request tracking integrated with existing metadata structure.The
request_fromfield is properly added to the metadata while preserving the existing nested error structure. This maintains backward compatibility while adding the new tracking capability.wren-ai-service/src/web/v1/services/sql_question.py (2)
11-11: Import addition is correct.The BaseRequest import is properly added to support the inheritance changes.
17-17: Inheritance change follows standardization pattern.SqlQuestionRequest now correctly inherits from BaseRequest, centralizing common fields and reducing code duplication.
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 17-17: Too few public methods (1/2)
(R0903)
wren-ai-service/src/web/v1/services/chart_adjustment.py (2)
10-10: LGTM: Clean inheritance from BaseRequestThe import of
BaseRequestand the inheritance changes forChartAdjustmentRequestandStopChartAdjustmentRequestproperly consolidate common request fields and eliminate code duplication.Also applies to: 27-27, 39-39
107-107: Good addition of request trackingAdding
request_fromto the metadata ensures consistent tracking of request origin throughout the service.wren-ai-service/src/web/v1/services/instructions.py (4)
11-11: Consistent addition of request tracking infrastructureThe import of
BaseRequestandMetadataTraceable, along with the addition ofrequest_fromto the Event model, follows the established pattern for request source tracking.Also applies to: 33-33
51-51: Good propagation of request context in error handlingThe addition of
request_fromparameter to_handle_exceptionand its usage in the Event creation ensures that request origin is tracked even in error scenarios.Also applies to: 58-58
62-62: Proper inheritance from BaseRequestBoth
IndexRequestandDeleteRequestcorrectly inherit fromBaseRequest, removing field duplication and standardizing the request structure.Also applies to: 123-123
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 62-62: Too few public methods (1/2)
(R0903)
110-110: Complete request tracking implementationThe propagation of
request_fromthrough both success and error paths in theindexanddeletemethods ensures comprehensive request origin tracking.Also applies to: 118-118, 149-149, 156-156
wren-ai-service/src/web/v1/services/__init__.py (2)
1-1: Good addition of request tracking to metadataThe import of type annotations and addition of
request_fromto_error_metadataenables consistent request source tracking across all services using MetadataTraceable.Also applies to: 20-20
46-61: Well-designed BaseRequest classThe
BaseRequestclass effectively consolidates common request fields and provides:
- Proper encapsulation with private
_query_idand public property- Sensible defaults (request_from defaulting to "ui")
- Type safety with Literal constraints for request_from
- Standard fields needed across all service requests
This eliminates code duplication and standardizes request handling across services.
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 47-47: Too few public methods (1/2)
(R0903)
wren-ai-service/src/web/v1/services/relationship_recommendation.py (4)
11-11: Consistent refactoring to BaseRequest patternThe import changes, inheritance from
BaseRequestfor the Input class, and addition ofrequest_fromto the Resource model follow the established standardization pattern.Also applies to: 17-17, 31-31
50-50: Proper error handling with request trackingThe addition of
request_fromparameter to_handle_exceptionand its propagation to the cached Resource ensures request origin is tracked in error scenarios.Also applies to: 57-57
72-72: Good fix: Corrected field name from configuration to configurationsThis change fixes a bug where
request.configuration.languagewas incorrectly accessing a non-existent field. The correct field name inBaseRequestisconfigurations(plural).
82-82: Complete request tracking in recommend methodThe propagation of
request_fromthrough both success and error paths ensures comprehensive request origin tracking throughout the recommendation lifecycle.Also applies to: 90-90, 97-97
wren-ai-service/src/web/v1/services/sql_corrections.py (5)
10-10: LGTM: Correct import of BaseRequest components.The import aligns with the standardization effort to use
BaseRequestacross all service request models.
27-27: LGTM: Added request_from tracking to Event model.This addition enables consistent tracking of request origins in event data, supporting the standardization pattern seen across other services.
45-45: LGTM: Proper propagation of request_from in error handling.The
_handle_exceptionmethod correctly accepts and stores therequest_fromparameter, ensuring error events maintain request origin context.Also applies to: 53-53
57-61: LGTM: Successful refactor to BaseRequest inheritance.The
CorrectionRequestclass now inherits fromBaseRequest, which removes redundant field definitions (likeproject_id) and centralizes common request properties. This aligns with the broader standardization effort across all service modules.🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 57-57: Too few public methods (1/2)
(R0903)
117-117: LGTM: Consistent request_from propagation throughout request lifecycle.The
request_fromfield is properly propagated from the incoming request to both success and error event handling, ensuring complete traceability of request origins.Also applies to: 126-126, 134-134
wren-ai-service/src/web/v1/services/semantics_description.py (6)
12-12: LGTM: Correct import of BaseRequest components.The import supports the standardization to use
BaseRequestacross service request models.
28-28: LGTM: Added request_from tracking to Resource model.This addition maintains consistency with other services in tracking request origins.
45-45: LGTM: Proper request_from propagation in error handling.The
_handle_exceptionmethod correctly accepts and uses therequest_fromparameter, maintaining request origin context in error scenarios.Also applies to: 52-52
56-61: LGTM: Successful refactor to BaseRequest inheritance.The
GenerateRequestclass now inherits fromBaseRequest, removing redundant field definitions and centralizing common request properties.🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 56-56: Too few public methods (1/2)
(R0903)
67-67: LGTM: Corrected configuration field access.Good catch! The access was corrected from
request.configuration.languagetorequest.configurations.language, aligning with the plural field name defined inBaseRequest.
119-119: LGTM: Complete request_from propagation throughout request lifecycle.The
request_fromfield is properly set on successful completion and propagated to all exception handling paths, ensuring comprehensive request origin tracking.Also applies to: 126-126, 133-133
wren-ai-service/src/web/v1/services/ask.py (3)
11-11: LGTM: Correct import of BaseRequest components.The import supports the standardization effort across all service modules.
22-22: LGTM: Clean refactor to BaseRequest inheritance.All request classes (
AskRequest,StopAskRequest,AskFeedbackRequest,StopAskFeedbackRequest) now inherit fromBaseRequest, which removes the need for internal_query_idmanagement and other common fields. This centralizes common request functionality effectively.Also applies to: 37-37, 95-95, 107-107
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 22-22: Too few public methods (1/2)
(R0903)
184-184: LGTM: Added request_from to metadata for tracking.The
request_fromfield is now included in the metadata returned by bothaskandask_feedbackmethods, enabling consistent request origin tracking across the system.Also applies to: 697-697
wren-ai-service/src/web/v1/services/sql_pairs.py (5)
11-11: LGTM: Correct import of BaseRequest components.The import aligns with the standardization effort across service modules.
26-26: LGTM: Added request_from tracking to Event model.This addition maintains consistency with other services for request origin tracking.
43-43: LGTM: Proper request_from propagation in error handling.The
_handle_exceptionmethod correctly accepts and uses therequest_fromparameter for error event creation.Also applies to: 50-50
54-57: LGTM: Successful refactor to BaseRequest inheritance.Both
IndexRequestandDeleteRequestclasses now inherit fromBaseRequest, removing redundant field definitions and centralizing common request properties.Also applies to: 97-100
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 54-54: Too few public methods (1/2)
(R0903)
84-84: LGTM: Proper request_from propagation in most cases.The
request_fromfield is correctly propagated in the index method and exception handling paths.Also applies to: 92-92, 125-125
wren-ai-service/src/web/v1/services/question_recommendation.py (7)
12-12: LGTM: Import updated for BaseRequest refactoringThe import correctly brings in the new
BaseRequestbase class along with other required types.
28-28: LGTM: Added request source trackingThe
request_fromfield properly tracks whether requests originate from the UI or API, with a sensible default value.
49-49: LGTM: Error handling updated to propagate request sourceThe
_handle_exceptionmethod correctly accepts and stores therequest_fromparameter, ensuring error events maintain the original request source context.Also applies to: 56-56
226-226: LGTM: Request source tracking on successful completionProperly sets the
request_fromfield on the cached event after successful question recommendation generation.
234-234: LGTM: Consistent error handling with request sourceBoth exception handlers correctly propagate the
request_fromfield, ensuring error tracking maintains the original request context.Also applies to: 241-241
155-155:✅ Verification successful
Verify BaseRequest provides required fields
The inheritance change from
BaseModeltoBaseRequestaligns with the standardization effort. However, ensure thatBaseRequestprovides all the fields that were previously available (likeproject_idandconfigurations).
🏁 Script executed:
#!/bin/bash # Verify BaseRequest class definition and its fields ast-grep --pattern 'class BaseRequest($_) { $$$ }'Length of output: 54
🏁 Script executed:
#!/bin/bash # Search for BaseRequest class definition across the repository rg -n "class BaseRequest" --color=never # If found, show surrounding context for field definitions if rg -q "class BaseRequest" --color=never; then rg -n "class BaseRequest" -A 20 --color=never else echo "No class BaseRequest definition found; checking imports and usage…" # Search for BaseRequest references to locate its module rg -n "BaseRequest" --color=never fiLength of output: 2021
✅ BaseRequest includes required fields
Verified that inwren-ai-service/src/web/v1/services/__init__.py,BaseRequest(lines 47–52) defines bothproject_idandconfigurations(among others), so the switch fromBaseModeltoBaseRequestpreserves all needed fields.🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 155-155: Too few public methods (1/2)
(R0903)
173-173:✅ Verification successful
Verify configuration/configurations field compatibility
The change from
input.configurationtoinput.configurationssuggests a field name change inBaseRequest. Ensure that:
configurationshas the same interface as the previousconfigurationfield- The
.languageattribute access (line 192) is still valid- The parameter type compatibility with
_validate_questionmethod (line 68 expectsConfigurationtype)Also applies to: 192-192
🏁 Script executed:
#!/bin/bash # Check BaseRequest field definitions and Configuration type usage echo "=== BaseRequest field definitions ===" ast-grep --pattern 'class BaseRequest($_) { $$$ configurations: $_ $$$ }' echo "=== Configuration class definition ===" ast-grep --pattern 'class Configuration($_) { $$$ }' echo "=== Usage of configurations field ===" rg -A 3 "\.configurations" --type pyLength of output: 7043
🏁 Script executed:
#!/bin/bash echo "=== Models directory contents ===" ls wren-ai-service/src/web/v1/models echo echo "=== BaseRequest definition ===" rg -n "class BaseRequest" -n wren-ai-service/src/web/v1/models echo echo "=== Configuration class definition ===" rg -n "class Configuration" -n wren-ai-service/src/web/v1/models echo echo "=== configuration field in BaseRequest ===" rg -n "configuration" -n wren-ai-service/src/web/v1/models/base_request.py || true echo echo "=== configurations field in BaseRequest ===" rg -n "configurations" -n wren-ai-service/src/web/v1/models/base_request.py || trueLength of output: 1203
🏁 Script executed:
#!/bin/bash echo "=== Searching for BaseRequest across repository ===" rg --type py -n "class BaseRequest" echo echo "=== Searching for Configuration class across repository ===" rg --type py -n "class Configuration"Length of output: 558
🏁 Script executed:
#!/bin/bash echo "=== BaseRequest snippet ===" rg -A10 "class BaseRequest" -n wren-ai-service/src/web/v1/services/__init__.py echo echo "=== Configuration snippet ===" rg -A20 "class Configuration" -n wren-ai-service/src/web/v1/services/__init__.py echo echo "=== _validate_question signature ===" rg -A5 -B5 "def _validate_question" -n wren-ai-service/src/web/v1/services/question_recommendation.py echo echo "=== Usages around input.configurations ===" rg -A5 -B5 "input.configurations" -n wren-ai-service/src/web/v1/services/question_recommendation.pyLength of output: 2776
🏁 Script executed:
#!/bin/bash echo "=== Full _validate_question signature ===" rg -A20 -B2 "async def _validate_question" -n wren-ai-service/src/web/v1/services/question_recommendation.pyLength of output: 1283
configurationsfield is correctly wired through the request and validation
All checks pass:
BaseRequestdefinesconfigurations: Configurationwith a defaultinput.configurations.languageis valid on theConfigurationmodel_validate_question(..., configuration: Configuration)is invoked positionally withinput.configurationsNo further changes needed.
| @@ -75,6 +64,7 @@ async def sql_question( | |||
| "error_type": "", | |||
| "error_message": "", | |||
| }, | |||
| "request_from": sql_question_request.request_from, | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent placement of request_from field.
The request_from field is added at the top level of the results dictionary, which is inconsistent with the pattern established in other services where it's placed within the metadata subdictionary (lines 63-66). This inconsistency could lead to confusion and processing issues.
Move the field to the metadata for consistency:
results = {
"sql_question_result": {},
"metadata": {
"error_type": "",
"error_message": "",
+ "request_from": sql_question_request.request_from,
},
- "request_from": sql_question_request.request_from,
}📝 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.
| "request_from": sql_question_request.request_from, | |
| results = { | |
| "sql_question_result": {}, | |
| "metadata": { | |
| "error_type": "", | |
| "error_message": "", | |
| "request_from": sql_question_request.request_from, | |
| }, | |
| } |
🤖 Prompt for AI Agents
In wren-ai-service/src/web/v1/services/sql_question.py at line 67, the
request_from field is incorrectly placed at the top level of the results
dictionary. To fix this, move the request_from field inside the existing
metadata subdictionary (lines 63-66) to maintain consistency with other services
and avoid confusion.
| self._cache[request.id] = self.Event( | ||
| id=request.id, | ||
| status="finished", | ||
| request_from=request.request_from, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add missing trace_id for consistency.
The Event creation in the delete method's success case is missing the trace_id parameter, which is inconsistent with the index method (line 83) and other services.
self._cache[request.id] = self.Event(
id=request.id,
status="finished",
+ trace_id=kwargs.get("trace_id"),
request_from=request.request_from,
)📝 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.
| self._cache[request.id] = self.Event( | |
| id=request.id, | |
| status="finished", | |
| request_from=request.request_from, | |
| ) | |
| self._cache[request.id] = self.Event( | |
| id=request.id, | |
| status="finished", | |
| trace_id=kwargs.get("trace_id"), | |
| request_from=request.request_from, | |
| ) |
🤖 Prompt for AI Agents
In wren-ai-service/src/web/v1/services/sql_pairs.py around lines 116 to 120, the
Event object created in the delete method is missing the trace_id parameter,
causing inconsistency with the index method and other services. Add the trace_id
attribute from the request to the Event constructor to ensure consistent
tracking and logging.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes