Optimize priority engine urgency checks and admin system stats querying#792
Optimize priority engine urgency checks and admin system stats querying#792RohanExploit wants to merge 1 commit into
Conversation
- `PriorityEngine._calculate_urgency`: Added substring pre-filtering (`any(k in text for k in keywords)`) before expensive regex `search()` execution. - `get_system_stats`: Replaced group by loop with a single SQLAlchemy aggregation query using `func.count()` and `func.sum(case(...))` to reduce database round trips and computation 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. |
📝 WalkthroughWalkthroughThe PR contains two backend refactorings: the priority engine optimizes keyword pre-filtering for urgency regex matching by replacing a loop-with-break with an ChangesBackend Refactorings
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
Pull request overview
This PR optimizes two backend hot paths: (1) the PriorityEngine’s urgency scoring loop to reduce unnecessary regex executions, and (2) the admin stats endpoint to compute multiple user metrics in a single SQL aggregate query.
Changes:
- Optimized
_calculate_urgencyby using a singleany(k in text for k in keywords)pre-check before runningregex.search. - Reworked
/admin/statsto return total/admin/active user counts via one SQLAlchemy aggregate query (count+sum(case(...))) instead of group-by + Python aggregation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| backend/routers/admin.py | Consolidates user stats into a single aggregate query and casts results to ints with NULL-safe defaults. |
| backend/priority_engine.py | Streamlines keyword pre-check logic to avoid redundant looping before regex evaluation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/routers/admin.py`:
- Line 51: Replace the Python boolean equality used in the SQLAlchemy
expression: locate the expression that builds the CASE for active users (the
Case/func.sum call referencing User.is_active) and change the predicate from
using "User.is_active == True" to the SQLAlchemy boolean predicate API
"User.is_active.is_(True)"; update the CASE call inside the same query (the one
producing .label('active')) so the ORM expression uses .is_(True) instead of ==
True.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 496d4c91-d7ac-4906-b6ea-344aa1a7a199
📒 Files selected for processing (2)
backend/priority_engine.pybackend/routers/admin.py
| result = db.query( | ||
| func.count(User.id).label('total'), | ||
| func.sum(case((User.role == UserRole.ADMIN, 1), else_=0)).label('admins'), | ||
| func.sum(case((User.is_active == True, 1), else_=0)).label('active') |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and show the relevant section around the referenced lines.
ls -la backend/routers/admin.py
nl -ba backend/routers/admin.py | sed -n '35,80p'
# Also search for get_system_stats to confirm context.
rg -n "def get_system_stats" -S backend/routers/admin.py || true
rg -n "get_system_stats" -S backend/routers/admin.pyRepository: RohanExploit/VishwaGuru
Length of output: 202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la backend/routers/admin.py
nl -ba backend/routers/admin.py | sed -n '35,80p'
rg -n "def get_system_stats" -S backend/routers/admin.py || true
rg -n "get_system_stats" -S backend/routers/admin.pyRepository: RohanExploit/VishwaGuru
Length of output: 202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la backend/routers/admin.py
nl -ba backend/routers/admin.py | sed -n '35,80p'
rg -n "def get_system_stats" -S backend/routers/admin.py || true
rg -n "get_system_stats" -S backend/routers/admin.pyRepository: RohanExploit/VishwaGuru
Length of output: 202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba backend/routers/admin.py | sed -n '45,60p'Repository: RohanExploit/VishwaGuru
Length of output: 110
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print a narrow window around the referenced line number without relying on `nl`
awk 'NR>=45 && NR<=60 {printf "%d\t%s\n", NR, $0}' backend/routers/admin.py
# Search for the exact boolean predicate patterns
rg -n "is_active\s*==\s*True|is_active\.is_\(\s*True\s*\)" backend/routers/admin.py || true
# Show the full matching expression (if present)
rg -n "func\.sum\(case\(" backend/routers/admin.py -n || trueRepository: RohanExploit/VishwaGuru
Length of output: 912
Use SQLAlchemy boolean predicate API instead of == True (Line 51).
Prefer .is_(True) over == True in ORM expressions to avoid E712 and keep the SQL intent explicit.
Suggested fix
- func.sum(case((User.is_active == True, 1), else_=0)).label('active')
+ func.sum(case((User.is_active.is_(True), 1), else_=0)).label('active')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func.sum(case((User.is_active == True, 1), else_=0)).label('active') | |
| func.sum(case((User.is_active.is_(True), 1), else_=0)).label('active') |
🧰 Tools
🪛 Ruff (0.15.13)
[error] 51-51: Avoid equality comparisons to True; use User.is_active: for truth checks
Replace with User.is_active
(E712)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routers/admin.py` at line 51, Replace the Python boolean equality
used in the SQLAlchemy expression: locate the expression that builds the CASE
for active users (the Case/func.sum call referencing User.is_active) and change
the predicate from using "User.is_active == True" to the SQLAlchemy boolean
predicate API "User.is_active.is_(True)"; update the CASE call inside the same
query (the one producing .label('active')) so the ORM expression uses .is_(True)
instead of == True.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Optimizes hot paths based on identified algorithmic learnings, specifically addressing loop inefficiencies and redundant database aggregations, improving execution speed while maintaining exact semantic correctness.
PR created automatically by Jules for task 1382703304350867349 started by @RohanExploit
Summary by cubic
Optimizes urgency checks in the priority engine and streamlines the admin system stats query for faster execution with no changes to results. Cuts CPU work in hot paths and reduces database round-trips.
_calculate_urgency: Added a substring pre-check (any(k in text for k in keywords)) to avoid expensive regex searches when no keywords match.get_system_stats: Replaced group-by + Python aggregation with a single SQLAlchemy aggregate usingcountandsum(case(...)), returning typed ints.Written for commit c4735c7. Summary will update on new commits. Review in cubic
Summary by CodeRabbit