12 KiB
Backend Improvement Report
1. No Authentication — Security
Severity: Critical
There is zero authentication on any endpoint. Any caller with network access can create, read, update, and delete all sleep session data. CORS configuration in app/main.py:18-24 restricts browser origins but does nothing to protect against direct HTTP clients (curl, scripts, etc.).
Recommended fix (incremental):
- Immediate: Add a single shared API key checked via an
Authorization: Bearer <key>header using a FastAPI dependency. The key is stored in an environment variable. - Longer term: Proper user authentication if the app ever becomes multi-user.
Example dependency:
from fastapi import Security, HTTPException
from fastapi.security import HTTPBearer, HTTPAuthorizationCredentials
bearer = HTTPBearer()
def verify_api_key(creds: HTTPAuthorizationCredentials = Security(bearer)):
if creds.credentials != settings.API_KEY:
raise HTTPException(status_code=401, detail="Unauthorized")
2. O(days × slots × sessions) Timetable Query — Performance
Severity: Critical
app/routers/stats.py:77-99 contains a triple-nested Python loop to build the timetable:
while current_date <= to_date.date():
for i in range(96): # 96 fifteen-minute slots per day
for s in sessions: # ALL sessions loaded into memory
if s.start_time <= slot_end and (s.end_time is None or s.end_time >= slot_start):
is_sleeping = True
break
With 30 days and 1,000 sessions this is ~2.88 million comparisons executed in Python. The comparison work should happen in the database.
Fix: For each day, run a single SQL query that returns the sessions overlapping that day's range, then build the 96 slots against only the relevant sessions. Alternatively, generate a CTE of slot boundaries and join against sleep_session in a single query. Even the naive "one query per day" approach is far faster than the current Python loop because the database uses the index (see item 4).
3. In-Python Aggregation in Other Stats Endpoints — Performance
Severity: High
The same load-everything-then-loop pattern appears in the other two stats endpoints:
/api/stats/daily-summary(stats.py:28-47): loads all sessions for the date range, then loops through each one in Python to compute per-day totals./api/stats/wake-sleep-times(stats.py:119-134): loads all sessions, then loops per day to find the first wakeup and last bedtime.
Both of these can be replaced with a single SQL GROUP BY query per endpoint:
daily-summary:SELECT DATE(start_time), SUM(...) FROM sleep_session WHERE ... GROUP BY DATE(start_time)wake-sleep-times:SELECT DATE(start_time), MIN(start_time), MAX(end_time) FROM sleep_session WHERE ... GROUP BY DATE(start_time)
This removes the O(sessions × days) Python loop entirely.
4. Missing Database Indexes — Performance
Severity: High
Every stats query and the session list query filter and sort on start_time and end_time, but neither column has an index in app/models.py.
Fix: Add indexes to the model:
from sqlmodel import Field, SQLModel, Index
class SleepSession(SQLModel, table=True):
...
# after the class definition:
Index("ix_sleep_session_start_time", SleepSession.__table__.c.start_time)
Index("ix_sleep_session_end_time", SleepSession.__table__.c.end_time)
Or declare them inline with sa_column:
start_time: datetime = Field(sa_column=Column(DateTime, index=True))
This is especially important for the timetable query once it is pushed into SQL.
5. Magic Return Types on Session End — Code Quality / Simplification
Severity: High
app/crud.py:57-82 returns Union[SleepSession, bool, str, None] where the string value is the magic sentinel "invalid":
# crud.py
if session is None:
return None
if session.end_time is not None:
return False
if ...:
return "invalid"
return session
The caller in app/routers/sessions.py:64-73 then has to chain if result is None, if result is False, if result == "invalid" checks. This is fragile, untestable without string matching, and breaks static analysis.
Fix: Define typed exceptions and raise them:
class SessionNotFoundError(Exception): pass
class SessionAlreadyEndedError(Exception): pass
class InvalidEndTimeError(Exception): pass
crud.py raises, sessions.py catches and maps to the appropriate HTTPException. The function return type becomes simply SleepSession.
6. Silent Error Suppression in Stats — Code Quality / Documentation
Severity: Medium
app/routers/stats.py:24-25 and 114-115 catch invalid query parameter values and silently return empty data:
try:
day_start_time = time.fromisoformat(day_start)
except ValueError:
return {"days": []} # client gets empty data with no explanation
The client has no way to distinguish "no data for this range" from "your parameters were malformed". This makes debugging difficult.
Fix: Return a 422 Unprocessable Entity with a descriptive message:
from fastapi import HTTPException
try:
day_start_time = time.fromisoformat(day_start)
except ValueError:
raise HTTPException(status_code=422, detail=f"Invalid day_start format: {day_start!r}")
Alternatively, use FastAPI's Query() with a regex validator so malformed values are rejected before the handler runs.
7. updated_at Field Never Updated — Bug
Severity: Medium
app/models.py declares an updated_at column, but app/crud.py:106-113 (the update handler) never sets it. After any PATCH, updated_at retains the creation timestamp, making it misleading.
Fix: In the update function, add:
session.updated_at = datetime.utcnow()
8. Dead Code — Code Slop
Severity: Medium
Three functions are defined but never called anywhere in the application:
| Function | File | Lines |
|---|---|---|
ensure_aware() |
app/crud.py |
7-10 |
minutes_after() |
app/utils/time_utils.py |
60-64 |
Additionally, app/utils/time_utils.py exports day_start_pattern() which is only used internally within get_day_night_splits() — it does not need to be a module-level export.
Fix: Delete ensure_aware() and minutes_after(). Make day_start_pattern() a local helper (leading underscore or inline).
9. Duplicate Test File — Code Slop
Severity: Medium
tests/test_sessions_fixed.py is an exact copy of tests/test_sessions.py. It appears to be a temporary file that was never cleaned up.
Fix: Delete tests/test_sessions_fixed.py.
10. No Application Logging — Observability
Severity: Medium
There are zero log statements anywhere in the backend. No requests, errors, slow queries, or business events are recorded. This makes diagnosing production issues entirely dependent on guesswork.
Fix: Add structured logging using Python's logging module (or structlog for structured output). At minimum log:
- Every request with method, path, and response status (can use a FastAPI middleware or Starlette's built-in
ServerErrorMiddleware) - Unhandled exceptions with stack traces
- Slow database queries (via SQLAlchemy event listeners if needed)
- Business events: session started, session ended, session deleted
Example setup in main.py:
import logging
logging.basicConfig(level=settings.LOG_LEVEL, format="%(asctime)s %(levelname)s %(name)s %(message)s")
logger = logging.getLogger(__name__)
Note: app/config.py already has a LOG_LEVEL setting — it is currently configured but never consumed.
11. No Rate Limiting — Security
Severity: Medium
There is no rate limiting on any endpoint. A client could spam POST requests to /api/sessions flooding the database, or hammer the expensive timetable endpoint.
Fix: Add slowapi (a FastAPI-compatible rate limiter):
from slowapi import Limiter
from slowapi.util import get_remote_address
limiter = Limiter(key_func=get_remote_address)
@router.post("/sessions")
@limiter.limit("10/minute")
async def create_session(...): ...
A reasonable starting limit: 10 writes/minute per IP, 60 reads/minute per IP.
12. Missing Test Coverage — Testing
Severity: High
Current test coverage has significant gaps:
| Area | Current state |
|---|---|
/api/stats/timetable |
1 basic happy-path test |
/api/stats/daily-summary |
1 basic happy-path test |
/api/stats/wake-sleep-times |
No tests |
| Date boundary conditions | No tests |
| Concurrent session creation | No tests |
| Invalid query parameters | No tests |
Recommended additions:
- Parametrised tests for all stats endpoints covering: empty date ranges, single session spanning midnight, session starting before range and ending inside it, multiple sessions on the same day.
- Test that invalid
day_start/day_endvalues return 422 (once fix #6 is applied). - Test that
limitparameter is bounded (once fix #14 is applied). - Regression test asserting
updated_atchanges after a PUT request (covers fix #7).
13. Unused Dependency — Code Slop
Severity: Low
python-multipart is listed in pyproject.toml:13 but is not used by this application (FastAPI only requires it if you use Form parameters, which this API does not).
Fix: Remove the dependency:
uv remove python-multipart
14. Unbounded limit Parameter — Security / Reliability
Severity: Medium
app/routers/sessions.py accepts a limit query parameter with default 200 and no upper bound. A caller can pass limit=10000000 and force a full table scan returning all rows.
Fix: Use FastAPI's Query with a le constraint:
from fastapi import Query
async def get_sessions(limit: int = Query(default=200, ge=1, le=1000)):
15. Hardcoded CORS Origin — Configuration
Severity: Low
app/config.py:7 hardcodes the CORS allowed origin:
CORS_ORIGINS: str = "http://localhost:5173"
This is already an env-var-overridable Settings field (via Pydantic's BaseSettings), so the mechanism is correct. But the default value means local development works without configuration while production requires the env var to be set — if it is forgotten, the production frontend gets CORS errors.
Documentation fix: Add a comment or a startup check that warns if the running environment looks like production but CORS_ORIGINS still has the localhost default.
Summary Table
| # | Category | File(s) | Severity |
|---|---|---|---|
| 1 | Security — no auth | All routers | Critical |
| 2 | Performance — O(n³) loop | stats.py:77-99 |
Critical |
| 3 | Performance — in-Python aggregation | stats.py:28-47, 119-134 |
High |
| 4 | Performance — missing indexes | models.py |
High |
| 5 | Code quality — magic return types | crud.py:57-82, sessions.py:64-73 |
High |
| 6 | Code quality — silent error suppression | stats.py:24-25, 114-115 |
Medium |
| 7 | Bug — updated_at not updated |
crud.py:106-113 |
Medium |
| 8 | Code slop — dead functions | crud.py:7-10, time_utils.py:60-64 |
Medium |
| 9 | Code slop — duplicate test file | tests/test_sessions_fixed.py |
Medium |
| 10 | Observability — no logging | All files | Medium |
| 11 | Security — no rate limiting | All routers | Medium |
| 12 | Testing — missing coverage | tests/ |
High |
| 13 | Code slop — unused dependency | pyproject.toml:13 |
Low |
| 14 | Security — unbounded limit param | sessions.py |
Medium |
| 15 | Configuration — hardcoded CORS | config.py:7 |
Low |