-
Notifications
You must be signed in to change notification settings - Fork 49
Prompt Issues Fixes #331
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
base: release
Are you sure you want to change the base?
Prompt Issues Fixes #331
Conversation
WalkthroughThe changes refine charting rules in the voice agent's system prompt by replacing generic weekly comparison labels with flexible single-series and period-over-period comparison rules, and introducing single-point handling guidance. A new error-handling rule for unavailable tools and data is also added. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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: 0
🧹 Nitpick comments (3)
app/agents/voice/automatic/prompts/system/charts.py (3)
82-85: Clarify the definition of "single-series line chart".While RULE 8 correctly addresses the PR objective of using actual dates for normal line charts, the term "single-series" could be ambiguous. Consider explicitly stating whether this means:
- One metric/line over time (vs multiple lines in RULE 9's period-over-period comparison)
- Or another specific criterion
This would help the LLM agent distinguish when to apply RULE 8 vs RULE 9 more reliably.
86-92: Consider edge case: period comparisons with single data points.RULE 9 handles period-over-period comparisons, but what happens when each period has only one data point? The precedence between RULE 9 (comparison) and RULE 10 (single point → single-stat) isn't explicit.
Consider adding guidance for this scenario, such as: "For comparisons where each period has only one data point, use a bar chart with period labels rather than a line chart or single-stat."
93-97: Clarify guidance for multiple metrics at a single time point.Line 96 states: "use bar/donut for comparison or Single-stat for primary metric" but the choice criteria between these options is unclear.
Consider providing explicit guidance such as:
- "If comparing multiple metrics: use bar chart"
- "If highlighting a primary metric with secondary context: use Single-stat"
This would reduce ambiguity for the LLM agent and ensure consistent chart selection.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/agents/voice/automatic/prompts/system/charts.py(1 hunks)app/agents/voice/automatic/prompts/system/tool_scope.py(1 hunks)
🔇 Additional comments (1)
app/agents/voice/automatic/prompts/system/tool_scope.py (1)
44-49: LGTM! Clear data availability rules added.The new "Tool & Data Availability" section effectively addresses the PR objective of preventing data fabrication. The directive to reject queries immediately when proper data is unavailable is unambiguous and includes helpful example phrases.
| 1. Exactly one (category, value) pair OR one time period with one metric → Single-stat chart | ||
| 2. Never use line/bar/donut charts for single data points (unless charting fails - see RULE 5) | ||
| 3. Multiple metrics at one time point ≠ time-series; use bar/donut for comparison or Single-stat for primary metric | ||
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.
swaroopvarma1
left a comment
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.
Please check comments
| 2. Graceful Degradation | ||
| For unrecoverable errors, apologize briefly ("Sorry, I encountered an issue.") and ask how to proceed. | ||
| 3. Tool & Data Availability | ||
| - If you do not have the appropriate tool or data to fulfill a request, politely reject it immediately |
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.
This can cause issues.
Example: If you ask what is my GMV for Euler only merchant they don't have any current GMV tool, but right now it can intelligently fetch GMV using other tools like payment analytics.
Such more similar cases may arise, which will limit user interactions.
Can you also share some sessions where it is causing issues currently for this fix.
Fixed Prompt Issues :
Summary by CodeRabbit
New Features
Bug Fixes