Skip to content

⚡ Bolt: Consolidated Aggregate Queries for Statistics #525

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.

Sign up for GitHub

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

Open
RohanExploit wants to merge 1 commit into
base: main
Choose a base branch
from
Open

⚡ Bolt: Consolidated Aggregate Queries for Statistics #525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .jules/bolt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,8 @@

## 2026-02-11 - Multi-Metric Aggregate Queries
**Learning:** Executing multiple separate `count()` queries to gather system statistics results in multiple database round-trips and redundant table scans.
**Action:** Use a single SQLAlchemy query with `func.count()` and `func.sum(case(...))` to calculate all metrics in one go. This reduces network overhead and allows the database to perform calculations in a single pass.
**Action:** Use a single SQLAlchemy query with `func.count()`, `func.sum(case(...))`, and `func.avg()` to calculate all metrics in one go. This reduces network overhead and allows the database to perform calculations in a single pass.

## 2026-02-28 - Label Mismatch in Aggregate Queries
**Learning:** When using `func.label()` in SQLAlchemy aggregate queries, any mismatch between the SQL label and the Python attribute accessed in the response dictionary will cause an `AttributeError`.
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This note references func.label(), but SQLAlchemy labeling is done with expression .label(...) / ColumnElement.label(). Update the wording to avoid documenting an incorrect API.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At .jules/bolt.md, line 58:

<comment>This note references `func.label()`, but SQLAlchemy labeling is done with expression `.label(...)` / `ColumnElement.label()`. Update the wording to avoid documenting an incorrect API.</comment>

<file context>
@@ -52,4 +52,8 @@
+**Action:** Use a single SQLAlchemy query with `func.count()`, `func.sum(case(...))`, and `func.avg()` to calculate all metrics in one go. This reduces network overhead and allows the database to perform calculations in a single pass.
+
+## 2026-02-28 - Label Mismatch in Aggregate Queries
+**Learning:** When using `func.label()` in SQLAlchemy aggregate queries, any mismatch between the SQL label and the Python attribute accessed in the response dictionary will cause an `AttributeError`.
+**Action:** Ensure SQL labels in aggregate queries exactly match the keys expected by the API response schema or dictionary mapping.
</file context>
Suggested change
**Learning:** When using `func.label()` in SQLAlchemy aggregate queries, any mismatch between the SQL label and the Python attribute accessed in the response dictionary will cause an `AttributeError`.
**Learning:** When using expression `.label()` (SQLAlchemy `ColumnElement.label()`) in aggregate queries, any mismatch between the SQL label and the Python attribute accessed in the response dictionary will cause an `AttributeError`.
Fix with Cubic

Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The journal entry says "When using func.label()" but SQLAlchemy labeling is done via the .label(...) method on an expression (e.g., func.count(...).label("name")). Using the wrong function name here could confuse readers trying to apply the guidance.

Suggested change
**Learning:** When using `func.label()` in SQLAlchemy aggregate queries, any mismatch between the SQL label and the Python attribute accessed in the response dictionary will cause an `AttributeError`.
**Learning:** When labeling expressions with `.label(...)` in SQLAlchemy aggregate queries, any mismatch between the SQL label and the Python attribute accessed in the response dictionary will cause an `AttributeError`.

Copilot uses AI. Check for mistakes.
**Action:** Ensure SQL labels in aggregate queries exactly match the keys expected by the API response schema or dictionary mapping.
Comment on lines +57 to +59
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

In SQLAlchemy, is func.label()a valid API for naming expressions, or should.label()be called on the SQL expression itself (for examplefunc.count(...).label("total"))?

💡 Result:

func.label() is not the intended API for aliasing/naming an expression in SQLAlchemy; func is for generating SQL function calls (e.g., COUNT(), COALESCE()). To name an expression, call .label() on the SQL expression (or use the standalone sqlalchemy.label() helper), e.g.:

from sqlalchemy import func, select

stmt = select(func.count(MyTable.id).label("total"))

SQLAlchemy’s docs describe labeling as a ColumnElement.label() method (and also provide the standalone label(name, element) function), which returns a Label (typically rendered with AS). [1] [2]

func.label() is the wrong API in this note.

SQLAlchemy labels are applied via .label() on the expression itself (e.g., func.count(...).label("total")). Documenting func.label() points readers at a non-existent API, since func is for generating SQL function calls, not for aliasing.

Suggested fix 
-**Learning:** When using `func.label()` in SQLAlchemy aggregate queries, any mismatch between the SQL label and the Python attribute accessed in the response dictionary will cause an `AttributeError`.
+**Learning:** When using `.label()` on SQLAlchemy expressions in aggregate queries (for example, `func.count(...).label("total")`), any mismatch between the SQL label and the Python attribute accessed in the response dictionary will cause an `AttributeError`.
📝 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.

Suggested change
## 2026-02-28 - Label Mismatch in Aggregate Queries
**Learning:** When using `func.label()` in SQLAlchemy aggregate queries, any mismatch between the SQL label and the Python attribute accessed in the response dictionary will cause an `AttributeError`.
**Action:** Ensure SQL labels in aggregate queries exactly match the keys expected by the API response schema or dictionary mapping.
## 2026-02-28 - Label Mismatch in Aggregate Queries
**Learning:** When using `.label()` on SQLAlchemy expressions in aggregate queries (for example, `func.count(...).label("total")`), any mismatch between the SQL label and the Python attribute accessed in the response dictionary will cause an `AttributeError`.
**Action:** Ensure SQL labels in aggregate queries exactly match the keys expected by the API response schema or dictionary mapping.
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.jules/bolt.md around lines 57 - 59, The note incorrectly references
func.label()—update the documentation to explain that labels are applied by
calling .label() on the SQL expression (e.g., call .label("total") on
func.count(...) or any column/expression) rather than on the func object;
replace any mentions of func.label() with the correct pattern such as
func.count(...).label("name") and clarify that the SQLAlchemy expression's
.label("key") must match the API/dictionary key expected.

31 changes: 14 additions & 17 deletions backend/routers/admin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
from fastapi import APIRouter, Depends, HTTPException, status
from fastapi import APIRouter, Depends
from sqlalchemy.orm import Session
from sqlalchemy import func, case
from typing import List

from sqlalchemy import func, case
from backend.database import get_db
from backend.models import User, UserRole
from backend.schemas import UserResponse
Expand All @@ -17,25 +16,23 @@

@router.get("/users", response_model=List[UserResponse])
def get_users(skip: int = 0, limit: int = 100, db: Session = Depends(get_db)):
users = db.query(User).offset(skip).limit(limit).all()
"""
Get all users with pagination.
Optimized: Uses column projection to reduce database overhead.
"""
users = db.query(
User.id,
User.email,
User.full_name,
User.role,
User.is_active,
User.created_at
).offset(skip).limit(limit).all()
Comment on lines +23 to +30
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add a deterministic sort before paginating.

Offset/limit over an unordered query is unstable, so /admin/users?skip= can duplicate or skip users as the planner or underlying data changes.

Suggested fix 
-    ).offset(skip).limit(limit).all()
+    ).order_by(User.id).offset(skip).limit(limit).all()
📝 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.

Suggested change
users = db.query(
User.id,
User.email,
User.full_name,
User.role,
User.is_active,
User.created_at
).offset(skip).limit(limit).all()
users = db.query(
User.id,
User.email,
User.full_name,
User.role,
User.is_active,
User.created_at
).order_by(User.id).offset(skip).limit(limit).all()
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@backend/routers/admin.py` around lines 23 - 30, The query building users =
db.query(User.id, User.email, User.full_name, User.role, User.is_active,
User.created_at).offset(skip).limit(limit).all() is missing a deterministic
order, causing unstable pagination; update the query that builds users to
include an explicit order_by (for example order by User.id or a stable compound
key like (User.created_at, User.id)) before applying offset/limit so pagination
is consistent across runs and data changes.

return users

@router.get("/stats")
def get_system_stats(db: Session = Depends(get_db)):
"""
Get system statistics.
Optimized: Uses a single database query with aggregations to avoid multiple aggregate round-trips.
"""
stats = db.query(
func.count(User.id).label("total_users"),
func.sum(case((User.role == UserRole.ADMIN, 1), else_=0)).label("admin_count"),
func.sum(case((User.is_active.is_(True), 1), else_=0)).label("active_users")
).first()

return {
"total_users": stats.total_users or 0,
"admin_count": stats.admin_count or 0,
"active_users": stats.active_users or 0,
Get system-wide user statistics.
Optimized: Uses a single aggregate query to calculate multiple metrics simultaneously,
reducing database round-trips and scan overhead.
Expand All @@ -45,7 +42,7 @@ def get_system_stats(db: Session = Depends(get_db)):
func.sum(case((User.role == UserRole.ADMIN, 1), else_=0)).label("admins"),
func.sum(case((User.is_active.is_(True), 1), else_=0)).label("active")
).first()

return {
"total_users": stats.total or 0,
"admin_count": int(stats.admins or 0),
Expand Down
51 changes: 18 additions & 33 deletions backend/routers/field_officer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,43 +403,28 @@ def get_issue_visit_history(
@router.get("/api/field-officer/visit-stats", response_model=VisitStatsResponse)
def get_visit_statistics(db: Session = Depends(get_db)):
"""
Get aggregate statistics for all field officer visits using optimized SQL queries

Returns metrics like total visits, verification status, geo-fence compliance, etc.
Get aggregate statistics for all field officer visits using optimized SQL queries.
Optimized: Consolidates 6 individual queries into a single aggregate query to reduce
database round-trips and scan overhead.
"""
try:
# Use SQL aggregates instead of loading all visits into memory
total_visits = db.query(func.count(FieldOfficerVisit.id)).scalar() or 0

verified_visits = db.query(func.count(FieldOfficerVisit.id)).filter(
FieldOfficerVisit.verified_at.isnot(None)
).scalar() or 0

within_geofence_count = db.query(func.count(FieldOfficerVisit.id)).filter(
FieldOfficerVisit.within_geofence == True
).scalar() or 0

outside_geofence_count = db.query(func.count(FieldOfficerVisit.id)).filter(
FieldOfficerVisit.within_geofence == False
).scalar() or 0

unique_officers = db.query(func.count(func.distinct(FieldOfficerVisit.officer_email))).scalar() or 0

average_distance = db.query(func.avg(FieldOfficerVisit.distance_from_site)).filter(
FieldOfficerVisit.distance_from_site.isnot(None)
).scalar()

# Round to 2 decimals if not None
if average_distance is not None:
average_distance = round(float(average_distance), 2)
# Performance Boost: Single aggregate query for all metrics
stats = db.query(
func.count(FieldOfficerVisit.id).label("total"),
func.sum(case((FieldOfficerVisit.verified_at.isnot(None), 1), else_=0)).label("verified"),
func.sum(case((FieldOfficerVisit.within_geofence == True, 1), else_=0)).label("within"),
func.sum(case((FieldOfficerVisit.within_geofence == False, 1), else_=0)).label("outside"),
Comment on lines +415 to +416
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Check if the file exists and examine the context around lines 415-416
if [ -f "backend/routers/field_officer.py" ]; then
  echo "=== File found, examining lines 410-420 ==="
  sed -n '410,420p' backend/routers/field_officer.py
  echo ""
  echo "=== Full context of the function (searching for function definition) ==="
  grep -n "def.*visit" backend/routers/field_officer.py | head -5
else
  echo "File not found"
fi

Repository: RohanExploit/VishwaGuru

Length of output: 1007

🏁 Script executed:

# Check for other boolean comparisons in the same file
echo "=== Searching for similar boolean comparisons in the file ==="
rg "== (True|False)" backend/routers/field_officer.py -n

Repository: RohanExploit/VishwaGuru

Length of output: 414

🌐 Web query:

Ruff E712 rule Python boolean comparison

💡 Result:

Ruff E712 (true-false-comparison) flags equality comparisons to the boolean singletons True / False (e.g., == True, != False) because they’re usually redundant or less clear than direct truthiness checks. [1][2]

What triggers E712

if cond == True:      # E712
    ...

if cond == False:     # E712
    ...

[2]

Preferred fixes

  • Truthiness check:
if cond:
    ...
  • Falsey check:
if not cond:
    ...

[1][2]

When an explicit boolean check is actually intended

If you specifically need to distinguish True/False from other truthy/falsey values (e.g., None, 0, ""), compare by identity:

if cond is True:
    ...

if cond is False:
    ...

This is also what the rule’s message recommends as an alternative. [2]

Sources: Ruff rule list (E712 description) [1], pycodestyle E712 documentation [2].

🌐 Web query:

SQLAlchemy .is_() method boolean predicates

💡 Result:

In SQLAlchemy, .is_() / .is_not() generate SQL IS / IS NOT predicates.

NULL predicates (most common)

IS NULL is what you usually want for None:

stmt = select(User).where(User.deleted_at.is_(None))      # deleted_at IS NULL
stmt = select(User).where(User.deleted_at.is_not(None))  # deleted_at IS NOT NULL

Note: col == None is special-cased by SQLAlchemy to also render col IS NULL automatically. (docs.sqlalchemy.org)

Boolean predicates

.is_() can be used for booleans too, and SQLAlchemy explicitly calls this out as a reason to use it (some platforms/dialects prefer IS TRUE/FALSE semantics). (docs.sqlalchemy.org)

Typical patterns:

from sqlalchemy import true, false

stmt = select(User).where(User.is_active.is_(true()))    # IS TRUE
stmt = select(User).where(User.is_active.is_(false()))   # IS FALSE

You can also do col.is_(True/False), but using true() / false() is the most explicit “SQL boolean constant” form. (This also avoids confusion with Python boolean comparisons.)

.is_not() vs .isnot()

Dialect caveat

Not every database has a native boolean type / boolean literals; behavior can vary by backend, so IS FALSE-style predicates may not be portable everywhere without dialect support. (github.com)

Citations:

Use SQLAlchemy boolean predicates instead of == True/False.

These comparisons will trip Ruff E712 here, and .is_(True) / .is_(False) keeps the generated SQL explicit.

Suggested fix 
-            func.sum(case((FieldOfficerVisit.within_geofence == True, 1), else_=0)).label("within"),
-            func.sum(case((FieldOfficerVisit.within_geofence == False, 1), else_=0)).label("outside"),
+            func.sum(case((FieldOfficerVisit.within_geofence.is_(True), 1), else_=0)).label("within"),
+            func.sum(case((FieldOfficerVisit.within_geofence.is_(False), 1), else_=0)).label("outside"),
📝 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.

Suggested change
func.sum(case((FieldOfficerVisit.within_geofence == True, 1), else_=0)).label("within"),
func.sum(case((FieldOfficerVisit.within_geofence == False, 1), else_=0)).label("outside"),
func.sum(case((FieldOfficerVisit.within_geofence.is_(True), 1), else_=0)).label("within"),
func.sum(case((FieldOfficerVisit.within_geofence.is_(False), 1), else_=0)).label("outside"),
🧰 Tools
🪛 Ruff (0.15.4)

[error] 415-415: Avoid equality comparisons to True; use FieldOfficerVisit.within_geofence: for truth checks

Replace with FieldOfficerVisit.within_geofence

(E712)

[error] 416-416: Avoid equality comparisons to False; use not FieldOfficerVisit.within_geofence: for false checks

Replace with not FieldOfficerVisit.within_geofence

(E712)

🤖 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 415 - 416, Replace the boolean
comparisons that use == True / == False in the aggregate expressions with
SQLAlchemy boolean predicates: change FieldOfficerVisit.within_geofence == True
to FieldOfficerVisit.within_geofence.is_(True) and
FieldOfficerVisit.within_geofence == False to
FieldOfficerVisit.within_geofence.is_(False) so the
func.sum(case(...)).label("within") and func.sum(case(...)).label("outside") use
.is_(True)/.is_(False) for explicit SQL and to avoid Ruff E712.

func.count(func.distinct(FieldOfficerVisit.officer_email)).label("unique_officers"),
func.avg(FieldOfficerVisit.distance_from_site).label("avg_dist")
).first()

return VisitStatsResponse(
total_visits=total_visits,
verified_visits=verified_visits,
within_geofence_count=within_geofence_count,
outside_geofence_count=outside_geofence_count,
unique_officers=unique_officers,
average_distance_from_site=average_distance
total_visits=stats.total or 0,
verified_visits=int(stats.verified or 0),
within_geofence_count=int(stats.within or 0),
outside_geofence_count=int(stats.outside or 0),
unique_officers=stats.unique_officers or 0,
average_distance_from_site=round(float(stats.avg_dist), 2) if stats.avg_dist is not None else None
)

except Exception as e:
Expand Down
20 changes: 15 additions & 5 deletions backend/routers/utility.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from fastapi import APIRouter, Depends, HTTPException, Query
from fastapi.responses import JSONResponse
from sqlalchemy.orm import Session
from sqlalchemy import func
from sqlalchemy import func, case
from datetime import datetime, timezone
import logging

Expand Down Expand Up @@ -49,16 +49,26 @@ def health():

@router.get("/api/stats", response_model=StatsResponse)
def get_stats(db: Session = Depends(get_db)):
"""
Get general issue statistics.
Optimized: Uses a single aggregate query for total and resolved counts to reduce
database round-trips.
"""
cached_stats = recent_issues_cache.get("stats")
if cached_stats:
return JSONResponse(content=cached_stats)

total = db.query(func.count(Issue.id)).scalar()
resolved = db.query(func.count(Issue.id)).filter(Issue.status.in_(['resolved', 'verified'])).scalar()
# Pending is everything else
# Performance Boost: Combined total and resolved count in one query
counts = db.query(
func.count(Issue.id).label("total"),
func.sum(case((Issue.status.in_(['resolved', 'verified']), 1), else_=0)).label("resolved")
).first()

total = counts.total or 0
resolved = int(counts.resolved or 0)
pending = total - resolved

# By category
# By category (separate group-by query is necessary here)
cat_counts = db.query(Issue.category, func.count(Issue.id)).group_by(Issue.category).all()
issues_by_category = {cat: count for cat, count in cat_counts}

Expand Down
124 changes: 124 additions & 0 deletions tests/test_bolt_stats.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
import pytest
from fastapi.testclient import TestClient
from backend.main import app
from backend.database import get_db, Base, engine
from backend.models import FieldOfficerVisit, User, UserRole
from sqlalchemy.orm import Session
from datetime import datetime, timezone

@pytest.fixture
def db_session():
Base.metadata.create_all(bind=engine)
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: The fixture mutates the shared application database engine instead of an isolated test database, so running this test can drop real tables.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_bolt_stats.py, line 11:

<comment>The fixture mutates the shared application database engine instead of an isolated test database, so running this test can drop real tables.</comment>

<file context>
@@ -0,0 +1,124 @@
+
+@pytest.fixture
+def db_session():
+    Base.metadata.create_all(bind=engine)
+    session = Session(bind=engine)
+    yield session
</file context>
Fix with Cubic

session = Session(bind=engine)
yield session
session.close()
Base.metadata.drop_all(bind=engine)
Comment on lines +4 to +15
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't run schema setup/teardown against the application engine.

This fixture imports backend.database.engine and then calls create_all/drop_all on it. If that engine is wired to anything except an isolated test database, the suite can mutate or drop real data. Build a dedicated test engine/sessionmaker inside the fixture instead of reusing the app engine.

#!/bin/bash
set -euo pipefail

# Expected: this should show an isolated test-only engine/URL, not the shared application engine.
fd '^database\.py$' backend --exec sed -n '1,220p' {}

echo
rg -n "create_engine|sessionmaker|DATABASE_URL|sqlite|postgres" backend tests
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/test_bolt_stats.py` around lines 4 - 15, The db_session fixture
currently uses the application engine (engine) and calls
Base.metadata.create_all / drop_all against it; replace that by creating a
dedicated test engine and sessionmaker inside the fixture (e.g.,
sqlalchemy.create_engine with an isolated test URL such as sqlite:///:memory: or
a test-specific DB) and bind a new Session from that test engine, then call
Base.metadata.create_all(bind=test_engine) and
Base.metadata.drop_all(bind=test_engine) and close the test session; update
symbols referenced: db_session fixture, Base, engine (stop using), and Session
(use sessionmaker(bind=test_engine) to produce sessions).


@pytest.fixture
def client(db_session):
app.dependency_overrides[get_db] = lambda: db_session
# Mock admin user for stats access
from backend.dependencies import get_current_admin_user
app.dependency_overrides[get_current_admin_user] = lambda: User(email="admin@example.com", role=UserRole.ADMIN)

with TestClient(app) as c:
yield c
app.dependency_overrides = {}
Comment on lines +18 to +26
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Expected: find a clear/delete/reset hook on the cache, or confirm that tests need to swap in a fresh cache instance.
fd '^cache\.py$' backend --exec sed -n '1,220p' {}

echo
rg -n 'recent_issues_cache|get\("stats"\)|set\([^)]*"stats"' backend tests

Repository: RohanExploit/VishwaGuru

Length of output: 7438

🏁 Script executed:

# Check if test_bolt_stats.py exists and view its contents
fd 'test_bolt_stats.py' tests --exec wc -l {} \;

echo "---"

# View the test file
fd 'test_bolt_stats.py' tests --exec cat -n {} \;

Repository: RohanExploit/VishwaGuru

Length of output: 5224

🏁 Script executed:

# Search for other test files that call /api/stats
rg '"/api/stats"' tests --type py

Repository: RohanExploit/VishwaGuru

Length of output: 131

Clear the global stats cache in the fixture.

The /api/stats endpoint returns cached data via recent_issues_cache.get("stats"), but this fixture only resets DB state and dependency overrides. Since recent_issues_cache has a 5-minute TTL and is never cleared, test_get_utility_stats becomes order-dependent if any earlier test primed the cache. Add recent_issues_cache.clear() before yielding the client.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/test_bolt_stats.py` around lines 18 - 26, The fixture client should
clear the global cache so tests aren't order-dependent: import the
recent_issues_cache symbol (the cache used by the /api/stats endpoint) and call
recent_issues_cache.clear() before yielding the TestClient in the client fixture
(and optionally call it again after the yield to fully reset state); update the
client fixture where get_db and get_current_admin_user are overridden to perform
this clear.


def test_get_visit_statistics(client, db_session):
# Add some visits
v1 = FieldOfficerVisit(
issue_id=1,
officer_email="off1@example.com",
officer_name="Officer 1",
Comment on lines +30 to +33
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test inserts FieldOfficerVisit rows that reference issue_id values without creating corresponding Issue rows. This passes on SQLite when foreign-key enforcement is off, but will fail on databases/environments where FK constraints are enforced (e.g., Postgres, or SQLite with PRAGMA foreign_keys=ON). Create the referenced Issue records (or explicitly disable FK checks for the test DB) to make the test portable and representative of production behavior.

Copilot uses AI. Check for mistakes.
check_in_latitude=19.0,
check_in_longitude=72.0,
check_in_time=datetime.now(timezone.utc),
verified_at=datetime.now(timezone.utc),
within_geofence=True,
distance_from_site=10.5,
status="verified"
)
v2 = FieldOfficerVisit(
issue_id=2,
officer_email="off1@example.com",
officer_name="Officer 1",
check_in_latitude=19.1,
check_in_longitude=72.1,
check_in_time=datetime.now(timezone.utc),
within_geofence=False,
distance_from_site=150.0,
status="checked_in"
)
v3 = FieldOfficerVisit(
issue_id=3,
officer_email="off2@example.com",
officer_name="Officer 2",
check_in_latitude=19.2,
check_in_longitude=72.2,
check_in_time=datetime.now(timezone.utc),
within_geofence=True,
distance_from_site=20.0,
status="checked_in"
)
db_session.add_all([v1, v2, v3])
db_session.commit()
Comment on lines +28 to +65
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Seed Issue rows before inserting FieldOfficerVisit records.

FieldOfficerVisit.issue_id is a real foreign key. Hardcoding 1/2/3 works only on backends that don't enforce it; backends with FK checks enabled will reject these inserts and make the test environment-dependent.

Suggested fix 
 def test_get_visit_statistics(client, db_session):
-    # Add some visits
+    from backend.models import Issue
+
+    i1 = Issue(description="Issue 1", category="Road", status="open")
+    i2 = Issue(description="Issue 2", category="Road", status="open")
+    i3 = Issue(description="Issue 3", category="Road", status="open")
+    db_session.add_all([i1, i2, i3])
+    db_session.flush()
+
     v1 = FieldOfficerVisit(
-        issue_id=1,
+        issue_id=i1.id,
         officer_email="off1@example.com",
         officer_name="Officer 1",
         check_in_latitude=19.0,
@@
     v2 = FieldOfficerVisit(
-        issue_id=2,
+        issue_id=i2.id,
         officer_email="off1@example.com",
         officer_name="Officer 1",
         check_in_latitude=19.1,
@@
     v3 = FieldOfficerVisit(
-        issue_id=3,
+        issue_id=i3.id,
         officer_email="off2@example.com",
         officer_name="Officer 2",
         check_in_latitude=19.2,
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/test_bolt_stats.py` around lines 28 - 65, The test
test_get_visit_statistics inserts FieldOfficerVisit rows using hardcoded
issue_id values but doesn't create corresponding Issue records; create and
persist Issue instances first (e.g., construct Issue objects for the same
issues, add them via db_session.add_all(...) and commit) or create Issue objects
and attach them to FieldOfficerVisit via the relationship before calling
db_session.add_all([v1, v2, v3]) and db_session.commit(), ensuring the
foreign-key constraint for FieldOfficerVisit.issue_id is satisfied.


response = client.get("/api/field-officer/visit-stats")
assert response.status_code == 200
data = response.json()

assert data["total_visits"] == 3
assert data["verified_visits"] == 1
assert data["within_geofence_count"] == 2
assert data["outside_geofence_count"] == 1
assert data["unique_officers"] == 2
# (10.5 + 150.0 + 20.0) / 3 = 180.5 / 3 = 60.1666... -> 60.17
assert data["average_distance_from_site"] == 60.17

def test_get_system_stats(client, db_session):
# UserRole.ADMIN is already imported
u1 = User(email="admin1@example.com", hashed_password="pw", role=UserRole.ADMIN, is_active=True)
u2 = User(email="user1@example.com", hashed_password="pw", role=UserRole.USER, is_active=True)
u3 = User(email="user2@example.com", hashed_password="pw", role=UserRole.USER, is_active=False)

db_session.add_all([u1, u2, u3])
db_session.commit()

response = client.get("/admin/stats")
assert response.status_code == 200
data = response.json()

assert data["total_users"] == 3
assert data["admin_count"] == 1
assert data["active_users"] == 2

def test_get_utility_stats(client, db_session):
from backend.models import Issue
i1 = Issue(description="Issue 1", category="Road", status="resolved")
i2 = Issue(description="Issue 2", category="Water", status="open")
i3 = Issue(description="Issue 3", category="Road", status="verified")

db_session.add_all([i1, i2, i3])
db_session.commit()

response = client.get("/api/stats")
assert response.status_code == 200
data = response.json()

assert data["total_issues"] == 3
assert data["resolved_issues"] == 2
assert data["pending_issues"] == 1
assert data["issues_by_category"]["Road"] == 2
assert data["issues_by_category"]["Water"] == 1

def test_get_users_admin(client, db_session):
u1 = User(email="u1@example.com", hashed_password="pw", role=UserRole.USER, is_active=True)
db_session.add(u1)
db_session.commit()

response = client.get("/admin/users")
assert response.status_code == 200
data = response.json()
assert len(data) >= 1
assert any(u["email"] == "u1@example.com" for u in data)