Refactor milestone answers statistics calculation (#229)

* Refactor milestone answers statistics calculation

- refactor statistics calculation
  - add `update_stats` function which updates milestones and milestone group statistics
    - optional `incremental` argument
      - if `True`, then statistics updated with any new answers since last calculation (as before)
      - if `False`, then recalculates all statistics using all answers (may be needed if e.g. some junk answers are deleted by an admin)
  - add apscheduler to schedule regular calls of this function (based on 271c75e)
    - add `STATS_CRONTAB` to app settings to allow the schedule to be set, with a default crontab of 3am every monday
  - add fastapi_injectable to allow use of fastapi dependencies outside of fastapi routes
  - reduce duplication
  - resolves #203
- `AnswerSession`
  - add `expired` flag: initially False
    - set to True by `get_or_create_current_milestone_answer_session` if it was created 7 or more days ago
    - set to True when stats are updated if it was created 9 days or more ago
      - includes a grace period to avoid setting a currently in use answer session to expired
    - once an answer session is expired, then answers can no longer be modified / submitted by the user
      - this should ensure that answers cannot be modified after they have been included in the statistics
    - resolves #219
  - add `included_in_statistics` flag: initially False
    - set to True once the answers from this session are included in the statistics
- `MilestoneAnswer`
  - remove `included_in_milestone_statistics` and `included_in_milestonegroup_statistics` flags
    - this is now done at the level of an answer session rather than for each individual answer
- milestone feedback functions
  - insert a `TrafficLight.invalid.value` instead of raising an exception if there are no statistics for a milestone id or group
  - no longer recalculate stats when constructing feedback to avoid slowing down a user request in this case

* update openapi.json & openapi-ts client

* add some comments and small fixes

* attempt fix for flaky test

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Harald Mack <harald.mack@protonmail.com>

Author: 3 people

docker-compose.yml

@@ -16,6 +16,7 @@ services:
       - PORT=${PORT:-80}
       - RELOAD=${RELOAD:-false}
       - LOG_LEVEL=${LOG_LEVEL:-info}
+      - STATS_CRONTAB=${STATS_CRONTAB:-"0 3 * * mon"}
     logging:
       driver: "local"
       options:

frontend/src/lib/client/schemas.gen.ts

@@ -543,15 +543,10 @@ export const MilestoneAgeScoreCollectionPublicSchema = {
             },
             type: 'array',
             title: 'Scores'
-        },
-        created_at: {
-            type: 'string',
-            format: 'date-time',
-            title: 'Created At'
         }
     },
     type: 'object',
-    required: ['milestone_id', 'expected_age', 'scores', 'created_at'],
+    required: ['milestone_id', 'expected_age', 'scores'],
     title: 'MilestoneAgeScoreCollectionPublic'
 } as const;
 

frontend/src/lib/client/types.gen.ts

@@ -137,7 +137,6 @@ export type MilestoneAgeScoreCollectionPublic = {
     milestone_id: number;
     expected_age: number;
     scores: Array<MilestoneAgeScore>;
-    created_at: string;
 };
 
 export type MilestoneAnswerPublic = {

mondey_backend/openapi.json

@@ -24,6 +24,8 @@ dependencies = [
     "webp",
     "python-dateutil",
     "checkdigit",
+    "apscheduler",
+    "fastapi-injectable",
 ]
 dynamic = ["version"]
 

mondey_backend/pyproject.toml

@@ -5,9 +5,12 @@
 from contextlib import asynccontextmanager
 
 import uvicorn
+from apscheduler.schedulers.asyncio import AsyncIOScheduler
+from apscheduler.triggers.cron import CronTrigger
 from fastapi import FastAPI
 from fastapi.middleware.cors import CORSMiddleware
 from fastapi.staticfiles import StaticFiles
+from fastapi_injectable.util import get_injected_obj
 
 from .databases.mondey import create_mondey_db_and_tables
 from .databases.users import create_user_db_and_tables
@@ -18,13 +21,25 @@
 from .routers import research
 from .routers import users
 from .settings import app_settings
+from .statistics import update_stats
+
+
+def scheduled_update_stats():
+    return get_injected_obj(update_stats)
 
 
 @asynccontextmanager
 async def lifespan(app: FastAPI):
     create_mondey_db_and_tables()
     await create_user_db_and_tables()
+    scheduler = AsyncIOScheduler()
+    scheduler.add_job(
+        scheduled_update_stats,
+        CronTrigger.from_crontab(app_settings.STATS_CRONTAB),
+    )
+    scheduler.start()
     yield
+    scheduler.shutdown()
 
 
 def create_app() -> FastAPI:

mondey_backend/src/mondey_backend/main.py

@@ -158,8 +158,6 @@ class MilestoneAnswer(SQLModel, table=True):
     )
     milestone_group_id: int = Field(default=None, foreign_key="milestonegroup.id")
     answer: int
-    included_in_milestone_statistics: bool = False
-    included_in_milestonegroup_statistics: bool = False
 
 
 class MilestoneAnswerSession(SQLModel, table=True):
@@ -171,6 +169,8 @@ class MilestoneAnswerSession(SQLModel, table=True):
             "server_default": text("CURRENT_TIMESTAMP"),
         }
     )
+    expired: bool
+    included_in_statistics: bool
     answers: Mapped[dict[int, MilestoneAnswer]] = dict_relationship(key="milestone_id")
 
 
@@ -214,7 +214,6 @@ class MilestoneAgeScoreCollectionPublic(SQLModel):
     milestone_id: int
     expected_age: int
     scores: list[MilestoneAgeScore]
-    created_at: datetime.datetime
 
 
 class MilestoneGroupAgeScore(SQLModel, table=True):
@@ -235,8 +234,3 @@ class MilestoneGroupAgeScoreCollection(SQLModel, table=True):
         default=None, primary_key=True, foreign_key="milestonegroup.id"
     )
     scores: Mapped[list[MilestoneGroupAgeScore]] = back_populates("collection")
-    created_at: datetime.datetime = Field(
-        sa_column_kwargs={
-            "server_default": text("CURRENT_TIMESTAMP"),
-        }
-    )

mondey_backend/src/mondey_backend/models/milestones.py

@@ -1,8 +1,6 @@
 from __future__ import annotations
 
 import logging
-from datetime import datetime
-from datetime import timedelta
 from enum import Enum
 from typing import cast
 
@@ -15,8 +13,6 @@
 from ..models.milestones import MilestoneAnswerSession
 from ..models.milestones import MilestoneGroupAgeScore
 from ..models.milestones import MilestoneGroupAgeScoreCollection
-from .statistics import calculate_milestone_statistics_by_age
-from .statistics import calculate_milestonegroup_statistics_by_age
 from .utils import get_child_age_in_months
 
 
@@ -88,7 +84,6 @@ def compute_milestonegroup_feedback_summary(
     by first calculating the mean score over all milestones that belong to the milestonegroup that
     are relevant for the child when the given answersession was created. The mean is then
     compared against the mean and standard deviation over the known population of children for the child's age.
-    When the statistics is outdated (older than a week currently) or there is none, it is recomputed and updated in the database.
     See `compute_feedback_simple` for the feedback logic.
 
     Parameters
@@ -123,53 +118,35 @@ def compute_milestonegroup_feedback_summary(
     logger.debug(f"  child age in months: {age}")
     # extract milestonegroups
     groups = set(answer.milestone_group_id for answer in answersession.answers.values())
-    today = datetime.now()
 
     # for each milestonegroup, get the statistics, compute the current mean, and compute the feedback
-    # if the statistics is older than a week, we update it with the current data
     feedback: dict[int, int] = {}
     for group in groups:
         logger.debug(f"  group: {group}")
         stats = session.get(MilestoneGroupAgeScoreCollection, group)
-        logger.debug(f"  old stats: {stats}")
-        if stats is not None:
+        if stats is None:
+            logger.debug("  no stats")
+            feedback[group] = TrafficLight.invalid.value
+        else:
+            logger.debug(f"  stats: {stats}")
             for i, score in enumerate(stats.scores):
                 if score.count > 0:
                     logger.debug(
-                        f"   old score: , {i}, {score.count}, {score.avg_score}, {score.stddev_score}"
+                        f"   score: , {i}, {score.count}, {score.avg_score}, {score.stddev_score}"
                     )
-
-        if stats is None or stats.created_at < today - timedelta(days=7):
-            new_stats = calculate_milestonegroup_statistics_by_age(session, group)
-
-            if new_stats is None:
-                raise ValueError("No statistics for milestone group: ", group)
-
-            # update stuff in database
-            for i, new_score in enumerate(new_stats.scores):
-                if new_score.count > 0:
-                    logger.debug(
-                        f"   new_score: , {i}, {new_score.count}, {new_score.avg_score}, {new_score.stddev_score}"
-                    )
-                session.merge(new_score)
-
-            session.merge(new_stats)
-            session.commit()
-            stats = new_stats
-
-        # extract the answers for the current milestone group
-        group_answers = [
-            answer.answer + 1
-            for answer in answersession.answers.values()
-            if answer.milestone_group_id == group
-        ]
-        logger.debug(
-            f'  group answers: , {group_answers}, "mean: ", {np.mean(group_answers)}'
-        )
-        # use the statistics recorded for a certain age as the basis for the feedback computation
-        feedback[group] = compute_feedback_simple(
-            stats.scores[age], float(np.mean(group_answers))
-        )
+            # extract the answers for the current milestone group
+            group_answers = [
+                answer.answer + 1
+                for answer in answersession.answers.values()
+                if answer.milestone_group_id == group
+            ]
+            logger.debug(
+                f'  group answers: , {group_answers}, "mean: ", {np.mean(group_answers)}'
+            )
+            # use the statistics recorded for a certain age as the basis for the feedback computation
+            feedback[group] = compute_feedback_simple(
+                stats.scores[age], float(np.mean(group_answers))
+            )
     logger.debug(f"summary feedback: {feedback}")
     return feedback
 
@@ -179,8 +156,8 @@ def compute_milestonegroup_feedback_detailed(
 ) -> dict[int, dict[int, int]]:
     """
     Compute the per-milestone (detailed) feedback for all answers in a given answersession.
-    This is done by comparing the given answer per milestone against the mean and standard deviation of the known population of children for the child's age. If this statistics is outdated (older than a week currently) or is
-    missing, it is recomputed and updated in the database. See `compute_feedback_simple` for the feedback logic.
+    This is done by comparing the given answer per milestone against the mean and standard deviation of the known population of children for the child's age.
+    See `compute_feedback_simple` for the feedback logic.
     Return a dictionary mapping milestonegroup -> [milestone -> feedback].
     Parameters
     ----------
@@ -214,49 +191,28 @@ def compute_milestonegroup_feedback_detailed(
 
     age = get_child_age_in_months(child, answersession.created_at)
     logger.debug(f"  child age in months: {age}")
-    today = datetime.today()
 
     # for each milestonegroup, get the statistics, compute the current mean, and compute the feedback
     feedback: dict[int, dict[int, int]] = {}
     for milestone_id, answer in answersession.answers.items():
-        # try to get statistics for the current milestone and update it if it's not there
-        # or is too old
+        logger.debug(f"  milestone id: {milestone_id}, answer: {answer.answer + 1}")
         stats = session.get(MilestoneAgeScoreCollection, milestone_id)
-        logger.debug(f"  old stats: {stats}")
-        if stats is not None:
+        logger.debug(f"  stats: {stats}")
+        if answer.milestone_group_id not in feedback:
+            feedback[answer.milestone_group_id] = {}
+        if stats is None:
+            feedback[answer.milestone_group_id][cast(int, answer.milestone_id)] = (
+                TrafficLight.invalid.value
+            )
+        else:
             for i, score in enumerate(stats.scores):
                 if score.count > 0:
                     logger.debug(
-                        f"   old score: {i}, {score.count}, {score.avg_score}, {score.stddev_score}"
+                        f"   score: {i}, {score.count}, {score.avg_score}, {score.stddev_score}"
                     )
-
-        if stats is None or stats.created_at < today - timedelta(days=7):
-            new_stats = calculate_milestone_statistics_by_age(session, milestone_id)
-
-            if new_stats is None:
-                raise ValueError(
-                    "No new statistics could be calculated for milestone: ",
-                    milestone_id,
-                )
-
-            # update stuff in database
-            for i, new_score in enumerate(new_stats.scores):
-                if new_score.count > 0:
-                    logger.debug(
-                        f"   new_score: , {i}, {new_score.count}, {new_score.avg_score}, {new_score.stddev_score}"
-                    )
-                session.merge(new_score)
-
-            session.merge(new_stats)
-            session.commit()
-            stats = new_stats
-
-        if answer.milestone_group_id not in feedback:
-            feedback[answer.milestone_group_id] = {}
-
-        feedback[answer.milestone_group_id][cast(int, answer.milestone_id)] = (
-            compute_feedback_simple(stats.scores[age], answer.answer + 1)
-        )
+            feedback[answer.milestone_group_id][cast(int, answer.milestone_id)] = (
+                compute_feedback_simple(stats.scores[age], answer.answer + 1)
+            )
 
     logger.debug(f" detailed feedback: {feedback}")