Conversation
WalkthroughThe update revises SQL generation system prompts by adding explicit instructions for handling ranking problems with the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AI_System
participant SQL_Generator
User->>AI_System: Request SQL for ranking problem
AI_System->>SQL_Generator: Generate reasoning plan
SQL_Generator->>SQL_Generator: Add DENSE_RANK() step
SQL_Generator->>SQL_Generator: Add WHERE clause filtering
SQL_Generator->>AI_System: Return SQL query with DENSE_RANK()
AI_System->>User: Provide SQL query result
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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 (
|
cyyeh
added
module/ai-service
![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
🧹 Nitpick comments (4)
wren-ai-service/src/pipelines/generation/utils/sql.py (4)
186-187: Condense step-2 into a single sentence for clarity
Splitting a single enumerated item across two lines creates visual noise and increases the risk of accidental re-numbering when this string is edited again.-2. Explicitly state the following information in the reasoning plan: -if the user puts any specific timeframe(e.g. YYYY-MM-DD) in the user's question, you will put the absolute time frame in the SQL query; -Otherwise, you will put the relative timeframe in the SQL query. +2. Explicitly state the following information in the reasoning plan: if the user specifies an absolute date (e.g., YYYY-MM-DD) use that absolute date in the SQL query; otherwise use a relative timeframe.
188-196: Ranking rule added – provide an example and disambiguate the filter condition
Great to see the explicitDENSE_RANK()guidance. Two small improvements would make the instruction unambiguous:
- Mention the mandatory
ORDER BYclause inside the window, otherwise the assistant may omit it.- Give a minimal example (
rank <= 3) so it’s crystal-clear how the subsequentWHEREfilter should look.-3. For the ranking problem, you should use the ranking function, `DENSE_RANK()` to rank the results and then use `WHERE` clause to filter the results. +3. For ranking problems, use the window function `DENSE_RANK() OVER (ORDER BY <sorting-column> DESC)` to assign ranks, and then filter with a `WHERE rank <= <N>` (or equivalent) clause.
252-253: Duplicate the clarification from the reasoning prompt to the SQL rules block
Nice consistency. Consider adding the sameORDER BY+ example here as suggested above so both sections stay aligned.
262-265: Minor wording tweak to avoid repetition
“follow the instructions strictly to generate the SQL query” and “follow the plan step by step strictly” repeat “strictly” – trim one for readability.-1. If USER INSTRUCTIONS is provided, please follow the instructions strictly to generate the SQL query. +1. If USER INSTRUCTIONS is provided, please follow them to generate the SQL query.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wren-ai-service/src/pipelines/generation/utils/sql.py(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
⏰ 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)
2 participants
add ranking rule
Summary by CodeRabbit