chore(wren-ai-service): improve ai service #1068
Conversation
cyyeh
added
module/ai-service
WalkthroughThis pull request introduces a new SQL execution utility across multiple files in the wren-ai-service project. A new Changes
Sequence DiagramsequenceDiagram
participant User
participant Justfile
participant RunSQLScript
participant WrenEngine
User->>Justfile: Invoke run-sql with parameters
Justfile->>RunSQLScript: Execute script with MDL path, data source
RunSQLScript->>WrenEngine: Load MDL, prepare query
WrenEngine-->>RunSQLScript: Return query results
RunSQLScript-->>User: Display interactive SQL prompt
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 1
🧹 Nitpick comments (1)
wren-ai-service/Justfile (1)
68-68: Potential inconsistency with naming convention
The function nameuse-wren-ui-as-engineconsistently uses hyphens, while many lines above use underscores for local commands. This isn't strictly an error, but you might want to confirm or unify the naming style for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
wren-ai-service/Justfile(1 hunks)wren-ai-service/demo/run_sql.py(1 hunks)wren-ai-service/demo/utils.py(3 hunks)wren-ai-service/src/pipelines/generation/utils/chart.py(1 hunks)
🔇 Additional comments (10)
wren-ai-service/demo/run_sql.py (6)
1-3: Imports are straightforward
The core dependencies argparse and json are relevant and well-structured. Ensure that any additional dependencies used in the code (e.g., pandas) are declared in your project's environment or requirements.txt/pyproject.toml.
7-35: Argument parsing and defaults look good
The arguments --mdl-path, --data-source, and --sample-dataset have sensible defaults and clear help text. This aids both discoverability and usability.
42-51: Robust file loading with error handling
Using FileNotFoundError and json.JSONDecodeError provides a clean user-facing message. Great job ensuring the script gracefully exits upon encountering these errors.
52-74: Handles user input gracefully
Your loop for collecting lines until a semicolon is user-friendly and covers a typical “quit” scenario with 'q'. The immediate exception handling around get_data_from_wren_engine call is also a good practice for interactive scripts.
78-79: Main guard for script execution
The if __name__ == "__main__": main() pattern is correctly used. This supports both direct script usage and importing from other modules.
4-4: Use local imports carefully
Importing from utils assumes demo scripts can access the root-level utils.py. Confirm that your environment and paths are set up so that this local import works properly when packaging or distributing.
wren-ai-service/src/pipelines/generation/utils/chart.py (1)
13-14: Clarifying empty data conditions
Explicitly instructing that an empty schema must be returned when data is unsuitable or empty is an improvement. This ensures consistent behavior for chart generation.
wren-ai-service/demo/utils.py (3)
67-67: Optional dataset parameter
Declaring dataset as optional adds flexibility. Verify there are no unintended side effects when dataset is None, especially in downstream _prepare_duckdb(dataset).
141-141: Clearer error messaging
Using response.text in the assertion for response.status_code is effective for showing raw error information. This makes debugging more straightforward if the response is malformed JSON or non-JSON.
164-164: Consistent assertion style
The status check also uses response.text. Keeping these assertions uniform throughout the file ensures consistent debugging and error reporting.
wren-ai-service/Justfile
Outdated
| poetry run python -m src.force_update_config | ||
|
|
||
| run-sql mdl_path="" data_source="" sameple_dataset="": |
There was a problem hiding this comment.
Typo in parameter name
The parameter is spelled sameple_dataset, instead of sample_dataset. This might cause confusion or errors when referencing the variable.
-run-sql mdl_path="" data_source="" sameple_dataset="":
+run-sql mdl_path="" data_source="" sample_dataset="":📝 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.
| run-sql mdl_path="" data_source="" sameple_dataset="": | |
| run-sql mdl_path="" data_source="" sample_dataset="": |
demo/run_sql.pyto test WrenSQL easily using command lineSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor