From 48afe6de96b5ec8b15c2ebc3ef9440632bd4cbd2 Mon Sep 17 00:00:00 2001
From: Daniele De Dominicis <37807398+Danielededo@users.noreply.github.com>
Date: Sun, 11 Oct 2026 11:29:29 +0200
Subject: [PATCH] perf: recognise identical files before parsing them

store_workout parsed every file before asking whether its exact bytes were
already stored. Parsing is most of a file's cost, so re-sending an export
spent as long as the first import storing nothing: 130 s for 150 FIT
activities, enough on its own to hit nginx's 120 s timeout. Check the hash
first, then parse only for the same-session check. The retry now takes 0.7 s.
---
 backend/app/services/workouts.py | 35 +++++++++++++++++++-------------
 backend/tests/test_duplicates.py | 18 ++++++++++++++++
 docs/DESIGN.md                   |  5 +++++
 3 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/backend/app/services/workouts.py b/backend/app/services/workouts.py
index df263fb..5fb9e16 100644
--- a/backend/app/services/workouts.py
+++ b/backend/app/services/workouts.py
@@ -38,21 +38,27 @@ def __init__(self, existing_id: int, reason: str) -> None:
         self.reason = reason
 
 
-def find_duplicate(db: Session, user_id: int, parsed, file_hash: str) -> tuple[int, str] | None:
-    """Find an existing workout that is the same session as `parsed`.
+def find_identical(db: Session, user_id: int, file_hash: str) -> tuple[int, str] | None:
+    """Find an existing workout stored from exactly these bytes.
 
-    Two questions, because they need different answers. Byte-identical files
-    are caught by their hash — exact, cheap, and enforced by the database.
-    The same ride exported from Garmin as TCX and from Strava as GPX has
-    different bytes and a different `source`, so it can only be recognised by
-    what it describes: same sport, starting at practically the same moment.
+    Exact, cheap, and enforced by the database — and answerable before the file
+    is parsed, which is what makes re-sending an export fast: parsing is most of
+    the cost of a file, and an export sent twice is almost entirely files
+    already here.
     """
     exact = db.execute(
         select(Workout.id).where(Workout.user_id == user_id, Workout.file_hash == file_hash)
     ).scalar_one_or_none()
-    if exact is not None:
-        return exact, "identical file"
+    return None if exact is None else (exact, "identical file")
 
+
+def find_same_session(db: Session, user_id: int, parsed) -> tuple[int, str] | None:
+    """Find an existing workout that is the same session as `parsed`.
+
+    The same ride exported from Garmin as TCX and from Strava as GPX has
+    different bytes and a different `source`, so it can only be recognised by
+    what it describes: same sport, starting at practically the same moment.
+    """
     window = timedelta(seconds=settings.duplicate_window_seconds)
     near = db.execute(
         select(Workout.id)
@@ -79,12 +85,13 @@ def store_workout(db: Session, user_id: int, filename: str | None, content: byte
     if db.execute(select(User.id).where(User.id == user_id)).scalar_one_or_none() is None:
         raise UserNotFoundError(user_id)
 
-    parsed = parse_file(filename, content)
-
     file_hash = hashlib.sha256(content).hexdigest()
-    duplicate = find_duplicate(db, user_id, parsed, file_hash)
-    if duplicate is not None:
-        raise DuplicateWorkoutError(*duplicate)
+    if (identical := find_identical(db, user_id, file_hash)) is not None:
+        raise DuplicateWorkoutError(*identical)
+
+    parsed = parse_file(filename, content)
+    if (same_session := find_same_session(db, user_id, parsed)) is not None:
+        raise DuplicateWorkoutError(*same_session)
 
     metrics = compute_metrics(parsed)
     workout = Workout(
diff --git a/backend/tests/test_duplicates.py b/backend/tests/test_duplicates.py
index 771f576..5209b6e 100644
--- a/backend/tests/test_duplicates.py
+++ b/backend/tests/test_duplicates.py
@@ -35,6 +35,24 @@ def test_the_same_bytes_are_rejected_under_a_different_name(client, user, sample
     assert "identical file" in second.json()["detail"]
 
 
+def test_identical_bytes_are_recognised_without_parsing(client, user, sample_gpx, monkeypatch):
+    """Parsing is most of the cost of a file, and a re-sent export is all repeats.
+
+    Measured on a 150-activity archive, the second send took 130 s with nothing
+    to store — long enough to time out at the proxy on its own.
+    """
+    assert upload(client, user["id"], sample_gpx, "run.gpx").status_code == 201
+
+    def parse_must_not_run(*_args, **_kwargs):
+        raise AssertionError("an identical file was parsed")
+
+    monkeypatch.setattr("app.services.workouts.parse_file", parse_must_not_run)
+    second = upload(client, user["id"], sample_gpx, "run-again.gpx")
+
+    assert second.status_code == 409
+    assert "identical file" in second.json()["detail"]
+
+
 def test_the_same_file_is_allowed_for_a_different_user(client, user, other_user, sample_gpx):
     """The hash is scoped per user: two people may ride together."""
     assert upload(client, user["id"], sample_gpx, "run.gpx").status_code == 201
diff --git a/docs/DESIGN.md b/docs/DESIGN.md
index 610593f..9a8fc7b 100644
--- a/docs/DESIGN.md
+++ b/docs/DESIGN.md
@@ -98,6 +98,11 @@ within `DUPLICATE_WINDOW_SECONDS` — which no constraint can express, so the
 check lives in the service layer. A brick session still works: a ride and a run
 at the same time are different sports.
 
+**The exact question is asked before the file is parsed.** Parsing is most of
+what a file costs, and an export sent a second time is almost entirely files
+already stored — so hashing first took re-sending a 150-activity export from
+130 s to under a second.
+
 **A backfill that can tell "not computed" from "computed, nothing there" only
 visits an activity once.** The heart-rate histogram records an empty value for an
 activity that carried no heart rate, so a strapless one is not rescanned on every