From 9145bf8e474c70af4d043f34a1ce5764b38e19f1 Mon Sep 17 00:00:00 2001 From: Peter Stockings Date: Fri, 2 Oct 2026 14:05:37 +1000 Subject: [PATCH] Persist best efforts during import and add resumable backfill --- app/__init__.py | 3 + app/models.py | 7 +- app/records.py | 63 +++++- app/routes.py | 2 +- app/services.py | 3 + docs/PERSONAL_RECORDS.md | 36 +++- .../e57a0f824c61_persist_best_efforts.py | 29 +++ tests/test_app.py | 184 +++++++++++++++++- tests/test_records.py | 6 +- 9 files changed, 317 insertions(+), 16 deletions(-) create mode 100644 migrations/versions/e57a0f824c61_persist_best_efforts.py diff --git a/app/__init__.py b/app/__init__.py index 1c783fe..04aa8b7 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -67,6 +67,9 @@ def create_app(config=None): from .sync import register_cli register_cli(app) + from .records import register_cli as register_records_cli + + register_records_cli(app) @login_manager.user_loader def load_user(user_id): diff --git a/app/models.py b/app/models.py index 5ac2b34..efd640f 100644 --- a/app/models.py +++ b/app/models.py @@ -54,8 +54,11 @@ class Activity(db.Model): metrics = db.Column(db.JSON, nullable=False, default=dict, server_default="{}") visibility = db.Column(db.String(12), nullable=False, default="private") share_token = db.Column(db.String(64), unique=True) - samples = db.Column(db.JSON, nullable=False, default=list) - laps = db.Column(db.JSON, nullable=False, default=list) + best_efforts = db.Column(db.JSON, nullable=False, default=list, server_default="[]") + efforts_version = db.Column(db.Integer, nullable=False, default=0, server_default="0") + has_route = db.Column(db.Boolean, nullable=False, default=False, server_default=db.false()) + samples = db.deferred(db.Column(db.JSON, nullable=False, default=list)) + laps = db.deferred(db.Column(db.JSON, nullable=False, default=list)) created_at = db.Column(db.DateTime(timezone=True), default=lambda: datetime.now(UTC)) __table_args__ = (db.UniqueConstraint("owner_id", "source_key"),) diff --git a/app/records.py b/app/records.py index b2b548d..31af3de 100644 --- a/app/records.py +++ b/app/records.py @@ -2,8 +2,11 @@ from bisect import bisect_left, bisect_right +import click + from .analytics import numeric +EFFORTS_VERSION = 1 DISTANCES = ( ("1 km", 1000), ("2 km", 2000), @@ -75,15 +78,26 @@ def best_efforts(activity): return [best[label] for label, _ in DISTANCES if label in best] +def persist_efforts(activity): + """Store derived data in the same transaction as the imported run.""" + activity.best_efforts = best_efforts(activity) + activity.efforts_version = EFFORTS_VERSION + activity.has_route = any(sample.get("point") for sample in activity.samples) + + def record_book(activities): - """Recompute in activity chronology so backfills and removals stay correct. + """Compare persisted efforts in chronology; never read samples here. First efforts establish a record. Strict improvements earn historical PR badges, which remain on the run after a later activity beats that record. """ records, runs = {}, {} for activity in sorted(activities, key=lambda row: (row.started_at, row.id)): - efforts = best_efforts(activity) + efforts = ( + [dict(effort) for effort in activity.best_efforts] + if activity.efforts_version == EFFORTS_VERSION and activity.sport == "running" + else [] + ) for effort in efforts: previous = records.get(effort["label"]) effort["pr"] = previous is None or effort["seconds"] < previous["seconds"] - 1e-6 @@ -91,3 +105,48 @@ def record_book(activities): records[effort["label"]] = dict(effort, activity=activity) runs[activity.id] = efforts return {"records": [records[label] for label, _ in DISTANCES if label in records], "runs": runs} + + +def register_cli(app): + @app.cli.command("backfill-efforts") + @click.option("--batch-size", default=25, type=click.IntRange(1, 1000), show_default=True) + @click.option("--owner-id", type=click.IntRange(1), help="Limit to one athlete.") + @click.option("--force", is_flag=True, help="Recalculate even current-version efforts.") + def backfill(batch_size, owner_id, force): + """Backfill persisted best efforts in bounded, resumable transactions.""" + from sqlalchemy.orm import undefer + + from .extensions import db + from .models import Activity + + last_id, count = 0, 0 + # A fixed upper bound prevents concurrent imports extending this run forever. + maximum = db.session.scalar(db.select(db.func.max(Activity.id))) or 0 + while last_id < maximum: + query = db.select(Activity).where(Activity.id > last_id, Activity.id <= maximum) + if owner_id: + query = query.where(Activity.owner_id == owner_id) + if not force: + query = query.where(Activity.efforts_version != EFFORTS_VERSION) + rows = db.session.scalars( + query.options(undefer(Activity.samples)) + .order_by(Activity.id) + .limit(batch_size) + .with_for_update() + ).all() + if not rows: + break + try: + for row in rows: + persist_efforts(row) + last_id = rows[-1].id + db.session.commit() + except Exception as error: + db.session.rollback() + raise click.ClickException( + "Backfill failed; completed batches are saved. Rerun to resume." + ) from error + count += len(rows) + db.session.expunge_all() + click.echo(f"Backfilled {count} activities (through ID {last_id}).") + click.echo(f"Complete: {count} activities backfilled; effort version {EFFORTS_VERSION}.") diff --git a/app/routes.py b/app/routes.py index 508727c..a2dad10 100644 --- a/app/routes.py +++ b/app/routes.py @@ -79,7 +79,7 @@ def latest_mapped_run(rows): ( row for row in rows - if row.sport == "running" and any(sample.get("point") for sample in row.samples) + if row.sport == "running" and row.has_route ), None, ) diff --git a/app/services.py b/app/services.py index c19b699..caf01e9 100644 --- a/app/services.py +++ b/app/services.py @@ -8,6 +8,7 @@ from garmin_sync.service import SyncError, complete, decode_fit from .extensions import db from .models import Activity +from .records import persist_efforts def number(value, default=0): @@ -110,6 +111,7 @@ def activity_from_fit(owner_id, source_key, documents, metadata=None, refresh=Fa } if existing: existing.samples, existing.laps, existing.metrics = samples, laps, metrics + persist_efforts(existing) return existing row = Activity( owner_id=owner_id, @@ -125,6 +127,7 @@ def activity_from_fit(owner_id, source_key, documents, metadata=None, refresh=Fa samples=samples, laps=laps, ) + persist_efforts(row) db.session.add(row) return row diff --git a/docs/PERSONAL_RECORDS.md b/docs/PERSONAL_RECORDS.md index 87dd094..ced459f 100644 --- a/docs/PERSONAL_RECORDS.md +++ b/docs/PERSONAL_RECORDS.md @@ -1,6 +1,6 @@ # Running personal records -The dashboard and owner's run page now calculate estimated best efforts for +Activity import calculates and persists estimated best efforts for 1 km, 2 km, 5 km, 10 km, half marathon (21,097.5 m), and marathon (42,195 m). These are fastest stretches anywhere within a run, rather than lap times or times projected from average pace. All existing running activities participate. @@ -22,8 +22,36 @@ These are estimates from watch data, not verified race records or an exact clone of Strava's proprietary calculation. GPS/distance spikes or sparse recording can affect results. There is currently no manual effort exclusion or correction UI. -The initial implementation computes on page requests without a database migration. -For large histories, the next step is to persist per-run efforts at import time -with an algorithm version, backfill existing runs, and query those small results. +Imports (FIT upload, Garmin sync and local cache) persist per-run efforts in the +same transaction as the run. Refreshing recorded data recalculates the efforts. +`efforts_version` identifies the calculation version; version 0 means pending. +A current version with an empty list means calculated with no eligible distances. +Page requests only compare stored efforts; there is no sample-based fallback. +Raw samples and laps are deferred in list queries. Route availability is also +persisted, so the dashboard can choose its map without loading historical samples. +Chronological PR comparisons still happen on page requests, using at most six +stored efforts per run, preserving correct badges after backfills or removals. + +After `flask --app app db upgrade`, run: + +```sh +flask --app app backfill-efforts --batch-size 25 +``` + +The command commits each bounded batch and skips current-version rows on reruns. +It includes non-running and incomplete activities, storing an empty result when +appropriate. `--owner-id ID` limits the operation to one athlete; `--force` +recalculates current results. It uses row locks on PostgreSQL to coordinate with +concurrent refreshes, and fixes an upper activity ID at the start so new imports +do not extend the backfill indefinitely. New imports calculate their own results. +No FIT files need to be downloaded again. On Dokku: + +```sh +dokku run stride flask --app app backfill-efforts --batch-size 25 +``` + +Until backfill finishes, pending activities do not contribute to records or +route selection. Keep backfills separate from schema migrations so a large +history does not delay every deployment. Further additions could include second/third-best medals, annual records, manual exclusion of bad recordings, and a selected-effort highlight on the route map. diff --git a/migrations/versions/e57a0f824c61_persist_best_efforts.py b/migrations/versions/e57a0f824c61_persist_best_efforts.py new file mode 100644 index 0000000..3f9e1b9 --- /dev/null +++ b/migrations/versions/e57a0f824c61_persist_best_efforts.py @@ -0,0 +1,29 @@ +"""Persist versioned running best efforts and route availability.""" + +import sqlalchemy as sa +from alembic import op + +revision = "e57a0f824c61" +down_revision = "d81b97ae5401" +branch_labels = None +depends_on = None + + +def upgrade(): + with op.batch_alter_table("activity") as batch_op: + batch_op.add_column( + sa.Column("best_efforts", sa.JSON(), nullable=False, server_default="[]") + ) + batch_op.add_column( + sa.Column("efforts_version", sa.Integer(), nullable=False, server_default="0") + ) + batch_op.add_column( + sa.Column("has_route", sa.Boolean(), nullable=False, server_default=sa.false()) + ) + + +def downgrade(): + with op.batch_alter_table("activity") as batch_op: + batch_op.drop_column("has_route") + batch_op.drop_column("efforts_version") + batch_op.drop_column("best_efforts") diff --git a/tests/test_app.py b/tests/test_app.py index 073f9a9..94b93d2 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -6,6 +6,7 @@ import pytest from app import create_app from app.extensions import db from app.models import Activity, Friendship, User +from app.records import EFFORTS_VERSION, persist_efforts from app.services import activity_data, import_cache @@ -34,6 +35,7 @@ def app(tmp_path): owner_id=1, source_key="test:1", title="A quiet morning run", + has_route=True, sport="running", started_at=datetime(2026, 9, 30, tzinfo=UTC), distance_m=5000, @@ -90,6 +92,7 @@ def test_personal_records_render_and_remain_owner_only(app): with app.app_context(): row = db.session.get(Activity, 1) row.samples = [{"distance": 0, "time": 0}, {"distance": 5000, "time": 1500}] + persist_efforts(row) row.share_token = "record-test" db.session.commit() client = app.test_client() @@ -106,25 +109,196 @@ def test_personal_records_render_and_remain_owner_only(app): assert b"5 km PR" not in client.get("/dashboard").data +def test_import_and_refresh_persist_efforts_in_the_run_transaction(app): + from app.services import activity_from_fit + + doc = { + "session": [{"sport": "running", "start_time": "2026-09-30T10:00:00Z"}], + "record": [ + {"timestamp": "2026-09-30T10:00:00Z", "distance": 0}, + {"timestamp": "2026-09-30T10:05:00Z", "distance": 1000}, + ], + } + with app.app_context(): + row = activity_from_fit(1, "test:new", [doc]) + db.session.commit() + row_id = row.id + db.session.remove() + row = db.session.get(Activity, row_id) + assert row.efforts_version == EFFORTS_VERSION + assert row.best_efforts[0]["seconds"] == 300 + assert activity_from_fit(1, "test:new", [doc]) is None + doc["record"][1]["timestamp"] = "2026-09-30T10:04:00Z" + activity_from_fit(1, "test:new", [doc], refresh=True) + db.session.commit() + assert row.best_efforts[0]["seconds"] == 240 + doc["record"] = [] + activity_from_fit(1, "test:new", [doc], refresh=True) + db.session.commit() + assert row.best_efforts == [] + assert row.efforts_version == EFFORTS_VERSION + + +def test_backfill_is_resumable_and_dashboard_uses_only_stored_efforts(app, monkeypatch): + from sqlalchemy import event + + from app import records + + with app.app_context(): + row = db.session.get(Activity, 1) + row.samples = [ + {"distance": 0, "time": 0, "point": [-33, 151]}, + {"distance": 5000, "time": 1500}, + ] + db.session.add( + Activity( + owner_id=1, + source_key="old:2", + title="Old ride", + sport="cycling", + started_at=row.started_at, + samples=[], + ) + ) + db.session.commit() + runner = app.test_cli_runner() + result = runner.invoke(args=["backfill-efforts", "--batch-size", "1"]) + assert result.exit_code == 0, result.output + assert "2 activities backfilled" in result.output + result = runner.invoke(args=["backfill-efforts"]) + assert result.exit_code == 0 + assert "0 activities backfilled" in result.output + with app.app_context(): + assert db.session.get(Activity, 2).best_efforts == [] + assert db.session.get(Activity, 2).efforts_version == EFFORTS_VERSION + statements = [] + + def capture(_conn, _cursor, statement, _params, _context, _many): + statements.append(statement) + + event.listen(db.engine, "before_cursor_execute", capture) + engine = db.engine + + def fail_calculation(_row): + raise AssertionError("Page requests must never calculate from raw samples") + + monkeypatch.setattr(records, "best_efforts", fail_calculation) + try: + client = app.test_client() + login(client) + assert b"5 km PR" in client.get("/dashboard").data + assert all("activity.samples" not in s and "activity.laps" not in s for s in statements) + assert b"Best efforts on this run" in client.get("/activities/1").data + finally: + event.remove(engine, "before_cursor_execute", capture) + + +def test_backfill_keeps_completed_batches_after_failure(app, monkeypatch): + from app import records + + with app.app_context(): + db.session.add( + Activity( + owner_id=2, + source_key="old:2", + title="Second athlete", + sport="running", + started_at=datetime(2026, 1, 1, tzinfo=UTC), + samples=[], + ) + ) + db.session.commit() + original = records.persist_efforts + + def fail_second(row): + if row.id == 2: + raise ValueError("Interrupted batch") + original(row) + + monkeypatch.setattr(records, "persist_efforts", fail_second) + runner = app.test_cli_runner() + result = runner.invoke(args=["backfill-efforts", "--batch-size", "1"]) + assert result.exit_code != 0 + with app.app_context(): + assert db.session.get(Activity, 1).efforts_version == EFFORTS_VERSION + assert db.session.get(Activity, 2).efforts_version == 0 + monkeypatch.setattr(records, "persist_efforts", original) + result = runner.invoke(args=["backfill-efforts", "--owner-id", "1"]) + assert result.exit_code == 0 + assert "0 activities backfilled" in result.output + result = runner.invoke(args=["backfill-efforts", "--batch-size", "1"]) + assert result.exit_code == 0 + assert "1 activities backfilled" in result.output + result = runner.invoke(args=["backfill-efforts", "--force", "--owner-id", "1"]) + assert result.exit_code == 0 + assert "1 activities backfilled" in result.output + + +def test_efforts_migration_upgrades_existing_schema(tmp_path): + from sqlalchemy import text + + application = create_app( + { + "TESTING": True, + "SECRET_KEY": "test", + "SQLALCHEMY_DATABASE_URI": f"sqlite:///{tmp_path / 'migration.db'}", + "RATELIMIT_ENABLED": False, + } + ) + runner = application.test_cli_runner() + result = runner.invoke(args=["db", "upgrade", "d81b97ae5401"]) + assert result.exit_code == 0, result.output + with application.app_context(): + db.session.execute( + text( + "INSERT INTO user (id, username, email, display_name, password_hash, timezone) " + "VALUES (1, 'test', 'test@example.test', 'Test', 'test', 'UTC')" + ) + ) + db.session.execute( + text( + "INSERT INTO activity (owner_id, source_key, title, sport, started_at, " + "distance_m, duration_s, elevation_m, visibility, samples, laps) " + "VALUES (1, 'old', 'Old run', 'running', '2026-01-01', " + "1000, 300, 0, 'private', '[]', '[]')" + ) + ) + db.session.commit() + result = runner.invoke(args=["db", "upgrade"]) + assert result.exit_code == 0, result.output + with application.app_context(): + row = db.session.get(Activity, 1) + assert row.best_efforts == [] + assert row.efforts_version == 0 + assert row.has_route is False + assert row.title == "Old run" + result = runner.invoke(args=["backfill-efforts"]) + assert result.exit_code == 0, result.output + with application.app_context(): + assert db.session.get(Activity, 1).efforts_version == EFFORTS_VERSION + + def test_dashboard_replays_latest_accessible_gps_run(app): client = app.test_client() login(client) page = client.get("/dashboard").data assert b'id="dashboard-route-map" data-url="/activities/1/data"' in page assert b'id="toggle-route-replay"' not in page - assert b'vendor/leaflet.js' in page + assert b"vendor/leaflet.js" in page assert b'class="dashboard-map"' not in page client.post("/logout") login(client, "other") page = client.get("/dashboard").data - assert b'No GPS running route available yet.' in page - assert b'/activities/1/data' not in page + assert b"No GPS running route available yet." in page + assert b"/activities/1/data" not in page with app.app_context(): - db.session.get(Activity, 1).samples = [] + row = db.session.get(Activity, 1) + row.samples = [] + persist_efforts(row) db.session.commit() client.post("/logout") login(client) - assert b'No GPS running route available yet.' in client.get("/dashboard").data + assert b"No GPS running route available yet." in client.get("/dashboard").data def test_friend_access_requires_acceptance_and_revokes_immediately(app): diff --git a/tests/test_records.py b/tests/test_records.py index 0d2dc09..51f56af 100644 --- a/tests/test_records.py +++ b/tests/test_records.py @@ -3,16 +3,18 @@ from types import SimpleNamespace import pytest -from app.records import best_efforts, record_book +from app.records import best_efforts, persist_efforts, record_book def run(points, id=1, sport="running"): - return SimpleNamespace( + activity = SimpleNamespace( id=id, sport=sport, started_at=datetime(2026, 1, 1, tzinfo=UTC) + timedelta(days=id), samples=[{"distance": d, "time": t} for d, t in points], ) + persist_efforts(activity) + return activity def test_fastest_window_can_start_between_samples():