-
Notifications
You must be signed in to change notification settings - Fork 33
Prompt Reasoning #367
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
Prompt Reasoning #367
Conversation
Completed Working on "Code Review"✅ Workflow completed successfully. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe analysis agent's embedded prompt was restructured to replace the "instructions_comments" field with a new "query_analysis" field and refine the "sql_query" field description with more explicit SQL generation constraints and continuation logic guidance. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
|
🚅 Deployed to the QueryWeaver-pr-367 environment in queryweaver
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
Pull request overview
This PR refines the analysis agent's prompt instructions to improve SQL query generation accuracy. The changes focus on replacing a vague "instructions_comments" field with a structured "query_analysis" field that enforces explicit reasoning about query construction decisions.
Key Changes:
- Replaced the open-ended "instructions_comments" field with a detailed "query_analysis" field that requires structured reasoning about columns, grain, metrics, aggregation, ranking, and filters
- Updated the "sql_query" field description to emphasize generating a single valid SQL query and clarified the rules for using previous answers
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/agents/analysis_agent.py (1)
258-258: Update reference to removed field.This line references
"instructions_comments"which was removed from the JSON schema (replaced by"query_analysis"at line 285). The model should be instructed to use the"explanation"field for commenting on instruction application issues.📝 Proposed fix
- - If you CANNOT apply instructions in the SQL, explain why under - "instructions_comments", "explanation" and reduce your confidence. + - If you CANNOT apply instructions in the SQL, explain why under + "explanation" and reduce your confidence.
🤖 Fix all issues with AI agents
In @api/agents/analysis_agent.py:
- Line 285: The long dictionary string value for the "query_analysis" key in
api/agents/analysis_agent.py exceeds pylint's 100-character line limit; split
the string into multiple shorter pieces (using implicit concatenation inside
parentheses or separate quoted strings joined with +) so the text content
remains identical but no single source line exceeds 100 characters, and update
the "query_analysis" entry accordingly (locate the "query_analysis" key in the
dict/structure in analysis_agent.py and break its value into multiple lines).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
api/agents/analysis_agent.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All Python code should pass pylint (use make lint)
Files:
api/agents/analysis_agent.py
🪛 GitHub Actions: Pylint
api/agents/analysis_agent.py
[error] 285-285: pylint: C0301 Line too long (781/100) detected at line 285. Exit code 16 from the pylint run: 'pipenv run pylint $(git ls-files '*.py')'.
⏰ 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). (1)
- GitHub Check: test
🔇 Additional comments (1)
api/agents/analysis_agent.py (1)
289-291: LGTM! Clearer SQL query instruction.The updated description provides more explicit guidance on SQL generation, clarifying that exactly one valid SQL query should be generated and that previous answers should only be used for continuation questions. This improves the prompt's clarity.
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.
Summary:
- Major: 1
Key themes:
- Prompt prose still references the removed
instructions_commentsfield, so the agent will emit an output schema the downstream parser no longer accepts.
Next steps:
- Reconcile the narrative instructions with the new
query_analysis/sql_queryschema (or restoreinstructions_comments) so the agent output contract remains self-consistent.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.