⚡ Bolt: [performance improvement] Optimize Stats Queries#545
⚡ Bolt: [performance improvement] Optimize Stats Queries#545RohanExploit wants to merge 1 commit intomainfrom
Conversation
Optimize database aggregate queries by combining multiple separate count/sum operations into a single query in field_officer.py and utility.py. This reduces the number of database round-trips and scan overhead.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
✅ Deploy Preview for fixmybharat canceled.
|
🙏 Thank you for your contribution, @RohanExploit!PR Details:
Quality Checklist:
Review Process:
Note: The maintainers will monitor code quality and ensure the overall project flow isn't broken. |
📝 WalkthroughWalkthroughTwo backend router endpoints consolidate multiple database queries into single optimized queries using labeled aggregates and case expressions, reducing database round-trips and improving query efficiency for visit statistics and issue status metrics. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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.
Pull request overview
This PR optimizes backend stats endpoints by collapsing multiple aggregate queries into single SQL aggregate queries to reduce DB round-trips.
Changes:
- Combine total/resolved issue counts into a single aggregate query in
/stats. - Combine multiple field officer visit aggregates (counts, distinct count, avg) into a single query in
/visit-stats.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| backend/routers/utility.py | Consolidates issue stats aggregation into one query (but currently missing an import needed for the new query). |
| backend/routers/field_officer.py | Consolidates visit statistics aggregates into a single SQL query. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| stats = db.query( | ||
| func.count(Issue.id).label("total"), | ||
| func.sum(case((Issue.status.in_(['resolved', 'verified']), 1), else_=0)).label("resolved") | ||
| ).first() |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/routers/utility.py">
<violation number="1" location="backend/routers/utility.py:58">
P1: Import `case` from SQLAlchemy before using it here; otherwise `/stats` will raise `NameError` at runtime.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| resolved = db.query(func.count(Issue.id)).filter(Issue.status.in_(['resolved', 'verified'])).scalar() | ||
| stats = db.query( | ||
| func.count(Issue.id).label("total"), | ||
| func.sum(case((Issue.status.in_(['resolved', 'verified']), 1), else_=0)).label("resolved") |
There was a problem hiding this comment.
P1: Import case from SQLAlchemy before using it here; otherwise /stats will raise NameError at runtime.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/routers/utility.py, line 58:
<comment>Import `case` from SQLAlchemy before using it here; otherwise `/stats` will raise `NameError` at runtime.</comment>
<file context>
@@ -53,8 +53,13 @@ def get_stats(db: Session = Depends(get_db)):
- resolved = db.query(func.count(Issue.id)).filter(Issue.status.in_(['resolved', 'verified'])).scalar()
+ stats = db.query(
+ func.count(Issue.id).label("total"),
+ func.sum(case((Issue.status.in_(['resolved', 'verified']), 1), else_=0)).label("resolved")
+ ).first()
+
</file context>
![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)
backend/routers/field_officer.py (1)
412-426: Good optimization consolidating multiple queries into one.The aggregation query correctly reduces database round-trips. However, for the boolean comparisons, consider using SQLAlchemy's
is_()method instead of== True/== Falseto satisfy linter rules while maintaining explicit SQL generation.♻️ Suggested refinement for boolean comparisons
stats = db.query( func.count(FieldOfficerVisit.id).label("total_visits"), func.sum(case((FieldOfficerVisit.verified_at.isnot(None), 1), else_=0)).label("verified_visits"), - func.sum(case((FieldOfficerVisit.within_geofence == True, 1), else_=0)).label("within_geofence_count"), - func.sum(case((FieldOfficerVisit.within_geofence == False, 1), else_=0)).label("outside_geofence_count"), + func.sum(case((FieldOfficerVisit.within_geofence.is_(True), 1), else_=0)).label("within_geofence_count"), + func.sum(case((FieldOfficerVisit.within_geofence.is_(False), 1), else_=0)).label("outside_geofence_count"), func.count(func.distinct(FieldOfficerVisit.officer_email)).label("unique_officers"), func.avg(FieldOfficerVisit.distance_from_site).label("average_distance") ).first()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/routers/field_officer.py` around lines 412 - 426, In the aggregation query building stats, replace the boolean comparisons that use "FieldOfficerVisit.within_geofence == True" and "== False" with SQLAlchemy's is_() to satisfy the linter and generate explicit SQL; e.g., change the case expressions in the db.query call to use FieldOfficerVisit.within_geofence.is_(True) and FieldOfficerVisit.within_geofence.is_(False) (the surrounding case(...) labels and other aggregates like total_visits, verified_visits, unique_officers, average_distance remain the same).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/routers/utility.py`:
- Around line 56-59: The stats query in utility.py uses SQLAlchemy's case
function (seen in the db.query call building stats with func.sum(case(...))) but
case is not imported, causing a NameError; fix by adding case to the SQLAlchemy
imports at the top of the file so the case symbol is available to the stats
query that references Issue, func.count and func.sum.
---
Nitpick comments:
In `@backend/routers/field_officer.py`:
- Around line 412-426: In the aggregation query building stats, replace the
boolean comparisons that use "FieldOfficerVisit.within_geofence == True" and "==
False" with SQLAlchemy's is_() to satisfy the linter and generate explicit SQL;
e.g., change the case expressions in the db.query call to use
FieldOfficerVisit.within_geofence.is_(True) and
FieldOfficerVisit.within_geofence.is_(False) (the surrounding case(...) labels
and other aggregates like total_visits, verified_visits, unique_officers,
average_distance remain the same).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1b3059cb-c76b-4613-bb3c-69a597dfa448
📒 Files selected for processing (2)
backend/routers/field_officer.pybackend/routers/utility.py
| stats = db.query( | ||
| func.count(Issue.id).label("total"), | ||
| func.sum(case((Issue.status.in_(['resolved', 'verified']), 1), else_=0)).label("resolved") | ||
| ).first() |
There was a problem hiding this comment.
Critical: case is not imported, causing a NameError at runtime.
The case function from SQLAlchemy is used on line 58 but is not imported. This will cause the /stats endpoint to fail.
🐛 Fix: Add `case` to the import statement
-from sqlalchemy import func
+from sqlalchemy import func, case🧰 Tools
🪛 Ruff (0.15.5)
[error] 58-58: Undefined name case
(F821)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/routers/utility.py` around lines 56 - 59, The stats query in
utility.py uses SQLAlchemy's case function (seen in the db.query call building
stats with func.sum(case(...))) but case is not imported, causing a NameError;
fix by adding case to the SQLAlchemy imports at the top of the file so the case
symbol is available to the stats query that references Issue, func.count and
func.sum.
perf: optimize stats queries
Optimize database aggregate queries by combining multiple separate count/sum operations into a single query in field_officer.py and utility.py. This reduces the number of database round-trips and scan overhead.
PR created automatically by Jules for task 69517637003798733 started by @RohanExploit
Summary by cubic
Optimized visit and issue stats by consolidating multiple DB queries into single aggregate queries, reducing round-trips and scan cost. No API changes; responses remain the same.
field_officer.get_visit_statisticsandutility.get_stats.Written for commit 0511394. Summary will update on new commits.
Summary by CodeRabbit