fix: prevent double-counting of sleep sessions in stats by grouping by start date
This commit is contained in:
+23
-18
@@ -113,22 +113,27 @@ async def update_session(
|
||||
return sleep_session
|
||||
|
||||
|
||||
async def get_all_time_stats(session: AsyncSession) -> tuple[int, float]:
|
||||
statement = select(func.count(), func.min(SleepSession.start_time))
|
||||
async def get_sleep_counts_by_start_date(
|
||||
session: AsyncSession,
|
||||
from_date: Optional[datetime] = None,
|
||||
to_date: Optional[datetime] = None,
|
||||
) -> List[tuple]:
|
||||
if to_date and to_date.hour == 0 and to_date.minute == 0 and to_date.second == 0 and to_date.microsecond == 0:
|
||||
to_date = to_date.replace(hour=23, minute=59, second=59, microsecond=999999)
|
||||
|
||||
query_to_date = to_date.replace(tzinfo=None) if to_date and to_date.tzinfo else to_date
|
||||
query_from_date = from_date.replace(tzinfo=None) if from_date and from_date.tzinfo else from_date
|
||||
|
||||
statement = (
|
||||
select(func.date(SleepSession.start_time), func.count())
|
||||
.group_by(func.date(SleepSession.start_time))
|
||||
.order_by(func.date(SleepSession.start_time).desc())
|
||||
)
|
||||
|
||||
if query_from_date:
|
||||
statement = statement.where(SleepSession.start_time >= query_from_date)
|
||||
if query_to_date:
|
||||
statement = statement.where(SleepSession.start_time <= query_to_date)
|
||||
|
||||
result = await session.exec(statement)
|
||||
total_sessions, first_session_time = result.one()
|
||||
|
||||
if total_sessions == 0 or first_session_time is None:
|
||||
return 0, 0.0
|
||||
|
||||
# DB has naive datetimes
|
||||
first_session_time_naive = first_session_time.replace(tzinfo=None)
|
||||
|
||||
# Get current date (naive for comparison)
|
||||
today_date = datetime.now().date()
|
||||
first_date = first_session_time_naive.date()
|
||||
|
||||
num_days = (today_date - first_date).days + 1
|
||||
avg_sleeps = total_sessions / num_days if num_days > 0 else 0.0
|
||||
|
||||
return total_sessions, avg_sleeps
|
||||
return list(result.all())
|
||||
|
||||
@@ -3,7 +3,7 @@ from typing import List, Optional
|
||||
from fastapi import APIRouter, Depends, Query
|
||||
from sqlmodel.ext.asyncio.session import AsyncSession
|
||||
from app.database import get_session
|
||||
from app.crud import get_sessions, get_all_time_stats
|
||||
from app.crud import get_sessions, get_sleep_counts_by_start_date
|
||||
from app.schemas import DailySummaryResponse, TimetableResponse, WakeupBedtimeResponse, SummaryResponse
|
||||
from app.utils.time_utils import get_day_night_splits
|
||||
|
||||
@@ -106,14 +106,26 @@ async def get_summary(
|
||||
to_date: datetime,
|
||||
session: AsyncSession = Depends(get_session),
|
||||
):
|
||||
sessions, _ = await get_sessions(session, from_date=from_date, to_date=to_date)
|
||||
total_sleeps = len(sessions)
|
||||
|
||||
# Period Stats
|
||||
period_counts = await get_sleep_counts_by_start_date(session, from_date=from_date, to_date=to_date)
|
||||
total_sleeps = sum(count for _, count in period_counts)
|
||||
num_days = (to_date.date() - from_date.date()).days + 1
|
||||
avg_sleeps_per_day = total_sleeps / num_days if num_days > 0 else 0.0
|
||||
|
||||
total_sleeps_all_time, avg_sleeps_per_day_all_time = await get_all_time_stats(session)
|
||||
|
||||
|
||||
# All-Time Stats
|
||||
all_counts = await get_sleep_counts_by_start_date(session)
|
||||
total_sleeps_all_time = sum(count for _, count in all_counts)
|
||||
avg_sleeps_per_day_all_time = 0.0
|
||||
if all_counts:
|
||||
# counts are ordered by desc date (from crud.py)
|
||||
# so last one is earliest.
|
||||
earliest_date_str = all_counts[-1][0]
|
||||
# SQLite date() returns YYYY-MM-DD string
|
||||
earliest_date = datetime.strptime(earliest_date_str, "%Y-%m-%d").date()
|
||||
today = datetime.now(timezone.utc).date()
|
||||
days_all_time = (today - earliest_date).days + 1
|
||||
avg_sleeps_per_day_all_time = total_sleeps_all_time / days_all_time if days_all_time > 0 else 0.0
|
||||
|
||||
return {
|
||||
"total_sleeps": total_sleeps,
|
||||
"avg_sleeps_per_day": avg_sleeps_per_day,
|
||||
|
||||
@@ -1,69 +1,44 @@
|
||||
import pytest
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from httpx import AsyncClient, ASGITransport
|
||||
from app.main import app
|
||||
from app.database import engine
|
||||
from app.models import SQLModel
|
||||
from app.models import SleepSession
|
||||
from sqlmodel.ext.asyncio.session import AsyncSession
|
||||
from app.database import engine, get_session
|
||||
from sqlmodel import SQLModel
|
||||
|
||||
@pytest.fixture
|
||||
async def setup_db():
|
||||
@pytest.mark.asyncio
|
||||
async def test_summary_double_counting_midnight(api_client):
|
||||
# Setup: Fresh database for this test
|
||||
async with engine.begin() as conn:
|
||||
await conn.run_sync(SQLModel.metadata.drop_all)
|
||||
await conn.run_sync(SQLModel.metadata.create_all)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_sessions_date_only_filter(setup_db):
|
||||
# Test that passing a date (midnight) also works and includes the whole day
|
||||
target_date = datetime(2023, 10, 1, tzinfo=timezone.utc)
|
||||
other_date = datetime(2023, 10, 2, tzinfo=timezone.utc)
|
||||
# Use a fresh session for insertion
|
||||
async for db_session in get_session():
|
||||
# 1. Setup: Insert a sleep session that spans across midnight
|
||||
start_time = datetime(2024, 1, 1, 23, 0)
|
||||
end_time = datetime(2024, 1, 2, 7, 0)
|
||||
|
||||
new_session = SleepSession(start_time=start_time, end_time=end_time)
|
||||
db_session.add(new_session)
|
||||
await db_session.commit()
|
||||
break # Exit after one iteration
|
||||
|
||||
# 2. Call the stats endpoint for a period that includes this session
|
||||
from_date = "2024-01-01T00:00:00"
|
||||
to_date = "2024-01-02T23:59:59"
|
||||
|
||||
async with AsyncClient(
|
||||
transport=ASGITransport(app=app), base_url="http://test"
|
||||
) as ac:
|
||||
# 1. Create a session that starts LATER in the day on OTHER date (should be excluded)
|
||||
# We end it immediately to avoid 409s in the test
|
||||
start_time_other = other_date.replace(hour=20, minute=0)
|
||||
resp_other = await ac.post("/api/sessions", json={"start_time": start_time_other.isoformat()})
|
||||
assert resp_other.status_code == 201
|
||||
session_id_other = resp_other.json()["id"]
|
||||
await ac.patch(f"/api/sessions/{session_id_other}/end", json={"end_time": (start_time_other + timedelta(minutes=30)).isoformat()})
|
||||
|
||||
# 2. Create a session that starts ON the target_date (should be included)
|
||||
start_time_target = target_date.replace(hour=20, minute=0)
|
||||
resp_target = await ac.post("/api/sessions", json={"start_time": start_time_target.isoformat()})
|
||||
assert resp_target.status_code == 201
|
||||
session_id_target = resp_target.json()["id"]
|
||||
await ac.patch(f"/api/sessions/{session_id_target}/end", json={"end_time": (start_time_target + timedelta(minutes=30)).isoformat()})
|
||||
|
||||
# 3. Create a session that starts EARLIER on the target_date (should be included)
|
||||
start_time_early = target_date.replace(hour=2, minute=0)
|
||||
resp_early = await ac.post("/api/sessions", json={"start_time": start_time_early.isoformat()})
|
||||
assert resp_early.status_code == 201
|
||||
session_id_early = resp_early.json()["id"]
|
||||
await ac.patch(f"/api/sessions/{session_id_early}/end", json={"end_time": (start_time_early + timedelta(minutes=30)).isoformat()})
|
||||
|
||||
# 4. Create a session that is on the OTHER date (should be excluded)
|
||||
start_time_other2 = other_date.replace(hour=2, minute=0)
|
||||
resp_other2 = await ac.post("/api/sessions", json={"start_time": start_time_other2.isoformat()})
|
||||
assert resp_other2.status_code == 201
|
||||
session_id_other2 = resp_other2.json()["id"]
|
||||
await ac.patch(f"/api/sessions/{session_id_other2}/end", json={"end_time": (start_time_other2 + timedelta(minutes=30)).isoformat()})
|
||||
response = await api_client.get(f"/api/stats/summary?from_date={from_date}&to_date={to_date}")
|
||||
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
|
||||
# 3. Verify that total_sleeps is exactly 1 for the requested period
|
||||
print(f"DEBUG: data={data}")
|
||||
assert data["total_sleeps"] == 1, f"Expected 1 sleep session, got {data['total_sleeps']}"
|
||||
|
||||
# 4. Verify that all-time stats also count this session exactly once
|
||||
assert data["total_sleeps_all_time"] == 1, f"Expected 1 all-time sleep session, got {data['total_sleeps_all_time']}"
|
||||
|
||||
|
||||
|
||||
# Query with to_date = target_date (midnight)
|
||||
# This should be expanded to 23:59:59 on target_date.
|
||||
# Should include the 2 sessions on target_date (target and early).
|
||||
# Should exclude the 2 sessions on other_date.
|
||||
to_date_str = target_date.strftime("%Y-%m-%d") # "2023-10-01"
|
||||
print(f"\nGET: to_date={to_date_str}")
|
||||
|
||||
response = await ac.get(f"/api/sessions?to_date={to_date_str}")
|
||||
print(f"GET RESPONSE: {response.status_code}, body={response.json()}")
|
||||
|
||||
assert response.status_code == 200
|
||||
sessions = response.json()["sessions"]
|
||||
|
||||
# We expect exactly 2 sessions (the ones on target_date)
|
||||
assert len(sessions) == 2, f"Expected 2 sessions (the ones on {to_date_str}), but got {len(sessions)}. (Body: {response.json()})"
|
||||
for s in sessions:
|
||||
# Verify that the sessions returned are indeed from the target_date
|
||||
assert target_date.strftime("%Y-%m-%d") in s["start_time"]
|
||||
|
||||
Reference in New Issue
Block a user