Repository navigation
feat(wren-ai-service): Add a few properties in Prediction Result for Evaluation - #1297
Conversation
paopa
added
module/ai-service
WalkthroughThis pull request implements several changes across evaluation modules. It marks the Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Eval as Evaluation Module
participant Pipeline as AskPipeline / Other Pipelines
participant SQLGen as SQLGenerationReasoning
User->>Eval: Send evaluation request
Eval->>Eval: Start timer (record start_time)
Eval->>Pipeline: Process evaluation request
alt AskPipeline flow
Pipeline->>SQLGen: Retrieve SQL reasoning data
SQLGen-->>Pipeline: Return reasoning output
end
Pipeline-->>Eval: Return prediction (with reasoning if applicable)
Eval->>Eval: Compute elapsed_time and update result
Eval-->>User: Return evaluation result with elapsed_time
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)
⏰ Context from checks skipped due to timeout of 90000ms (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
paopa
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
wren-ai-service/eval/pipelines.py (1)
358-382: 🛠️ Refactor suggestionConsider error handling for SQL reasoning.
The SQL reasoning step should include error handling to gracefully handle failures.
_reasoning = await self._sql_reasoner.run( query=prediction["input"], contexts=documents, sql_samples=prediction.get("samples", []) if self._allow_sql_samples else [], ) - reasoning = _reasoning.get("post_process", {}) + try: + reasoning = _reasoning.get("post_process", {}) + if not reasoning: + print(f"Warning: No reasoning generated for query: {prediction['input']}") + except Exception as e: + print(f"Error in SQL reasoning: {e}") + reasoning = {}
🧹 Nitpick comments (2)
wren-ai-service/eval/prediction.py (2)
108-108: Consider creating a tracking issue for the refactoring task.The TODO comment indicates technical debt that should be tracked properly.
Would you like me to help create an issue to track this refactoring task?
109-127: Consider extracting datasource configuration logic.The conditional logic for setting up different datasources could be moved to a dedicated configuration function for better maintainability.
- _mdl = base64.b64encode(orjson.dumps(dataset["mdl"])).decode("utf-8") - if "spider_" in path: - settings.datasource = "duckdb" - replace_wren_engine_env_variables( - "wren_engine", {"manifest": _mdl}, settings.config_path - ) - else: - _connection_info = base64.b64encode( - orjson.dumps(settings.bigquery_info) - ).decode("utf-8") - replace_wren_engine_env_variables( - "wren_ibis", - { - "manifest": _mdl, - "source": settings.datasource, - "connection_info": _connection_info, - }, - settings.config_path, - ) + def configure_datasource(path: str, dataset: dict, settings: EvalSettings) -> None: + _mdl = base64.b64encode(orjson.dumps(dataset["mdl"])).decode("utf-8") + if "spider_" in path: + settings.datasource = "duckdb" + replace_wren_engine_env_variables( + "wren_engine", {"manifest": _mdl}, settings.config_path + ) + else: + _connection_info = base64.b64encode( + orjson.dumps(settings.bigquery_info) + ).decode("utf-8") + replace_wren_engine_env_variables( + "wren_ibis", + { + "manifest": _mdl, + "source": settings.datasource, + "connection_info": _connection_info, + }, + settings.config_path, + ) + + configure_datasource(path, dataset, settings)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
wren-ai-service/eval/metrics/accuracy.py(2 hunks)wren-ai-service/eval/pipelines.py(10 hunks)wren-ai-service/eval/prediction.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: pytest
- GitHub Check: pytest
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (5)
wren-ai-service/eval/metrics/accuracy.py (1)
174-176: LGTM! Well-documented deprecation.The deprecation of
AccuracyMultiCandidateMetricis properly marked with a clear reason.wren-ai-service/eval/pipelines.py (4)
43-43: LGTM! Improved type safety.The type hint
list[dict]forextract_unitsparameter provides better type safety.
128-152: LGTM! Well-implemented timing metrics.The addition of elapsed time tracking provides valuable performance insights.
156-161: LGTM! Clear and informative docstring.The updated docstring for the
flatmethod provides clear context about its purpose and future plans.
333-335: LGTM! Enhanced SQL generation with reasoning.The addition of SQL reasoning step improves the generation pipeline.
5697e2b to
b45ac13
Compare
This PR introduces several improvements and cleanups to the SQL generation pipeline:
Key Changes
AccuracyMultiCandidateMetricas it's no longer needed for Text to SQL tasksAskPipelinewith dedicatedSQLGenerationReasoningcomponentTechnical Details
Pipeline Enhancements:
Code Quality:
extract_unitsfunction parametersflatmethodPerformance Tracking:
Summary by CodeRabbit
New Features
Refactor