feat(sync): split FillPendingDetails into activities/workouts phases

Progress becomes phase-aware ({Phase, Done, Total} instead of a flat {Done, Total}), with FullSync reporting an indeterminate "discovering" phase during backfill/incremental sync (there's no meaningful total before those calls have already happened), then FillPendingDetails reporting a real Done/Total for its activities pass, then its new, independent workouts pass. alignWorkoutTargets now takes a lap count instead of a []garmin.Lap slice, since it never used lap content, only length -- this lets the new workouts pass call it against laps read back from the DB rather than needing the original Garmin lap data again. Splitting the passes also fixes a latent bug: an activity whose details were fetched successfully but whose workout fetch failed in that same run previously had no way to ever retry the workout fetch, since ActivitiesMissingDetails stops returning it once details_fetched_at/splits_fetched_at are set. ActivitiesMissingWorkout queries workout_id/workout_raw_json independently, so it keeps surfacing that activity until its workout is actually fetched.
2026-07-27 06:55:23 +02:00
parent 32fdfc1c72
commit 35d9933d1b
4 changed files with 293 additions and 52 deletions
--- a/backend/internal/sync/service.go
+++ b/backend/internal/sync/service.go
@@ -54,9 +54,23 @@ func (c Config) withDefaults() Config {
 	return c
 }

-// Progress reports how far a currently-running (or just-finished)
-// FillPendingDetails pass has gotten, for a status banner to poll.
+// Phase values reported by Progress.Phase.
+const (
+	PhaseIdle        = "idle"
+	PhaseDiscovering = "discovering"
+	PhaseActivities  = "activities"
+	PhaseWorkouts    = "workouts"
+)
+
+// Progress reports how far a currently-running (or just-finished) FullSync
+// has gotten, for a status modal to poll. PhaseDiscovering (backfillCore/
+// incrementalSyncCore) has no meaningful Total -- discovering how many
+// activities exist IS the act of fetching them, so it's reported as an
+// indeterminate step (Done/Total both 0) rather than a fake percentage.
+// PhaseActivities/PhaseWorkouts do have a real Total, taken once from a
+// local DB count at the start of each pass (see FillPendingDetails).
 type Progress struct {
+	Phase string
 	Done  int
 	Total int
 }
@@ -80,19 +94,20 @@ func NewService(g garmin.Client, db *store.DB, userID int64, cfg Config, now fun
 	if now == nil {
 		now = time.Now
 	}
-	return &Service{garmin: g, db: db, userID: userID, cfg: cfg.withDefaults(), now: now}
+	return &Service{garmin: g, db: db, userID: userID, cfg: cfg.withDefaults(), now: now, progress: Progress{Phase: PhaseIdle}}
 }

-// Progress returns the current detail-fill progress (0/0 when idle).
+// Progress returns the current sync progress (phase idle, 0/0 when nothing
+// is running).
 func (s *Service) Progress() Progress {
 	s.progressMu.Lock()
 	defer s.progressMu.Unlock()
 	return s.progress
 }

-func (s *Service) setProgress(done, total int) {
+func (s *Service) setProgress(phase string, done, total int) {
 	s.progressMu.Lock()
-	s.progress = Progress{Done: done, Total: total}
+	s.progress = Progress{Phase: phase, Done: done, Total: total}
 	s.progressMu.Unlock()
 }

@@ -199,6 +214,11 @@ func (s *Service) FullSync(ctx context.Context, detailFillLimit int) error {
 	if err != nil {
 		return err
 	}
+	// Reset to idle on every return path, including an early error return
+	// from backfillCore/incrementalSyncCore before FillPendingDetails (which
+	// otherwise owns its own idle-reset) ever runs.
+	s.setProgress(PhaseDiscovering, 0, 0)
+	defer s.setProgress(PhaseIdle, 0, 0)

 	backfillCount, err := s.backfillCore(ctx)
 	if err != nil {
@@ -270,11 +290,22 @@ func (s *Service) fetchAndStoreWindow(ctx context.Context, startDate, endDate st
 	return len(activities), newCount, nil
 }

-// FillPendingDetails fetches get_activity_splits/get_activity_details for up
-// to limit activities that don't have them yet, then (re)classifies each one.
-// Calls are made sequentially with Config.InterCallDelay between them to
+// FillPendingDetails fetches activity details/splits for up to limit
+// activities missing them, then fetches workouts for up to limit activities
+// missing those (independently -- see ActivitiesMissingWorkout), then
+// (re)classifies every activity touched by the first pass. Each pass makes
+// its Garmin calls sequentially with Config.InterCallDelay between them to
 // avoid Garmin/Cloudflare rate limiting.
 func (s *Service) FillPendingDetails(ctx context.Context, limit int) error {
+	defer s.setProgress(PhaseIdle, 0, 0)
+
+	if err := s.fillPendingActivityDetails(ctx, limit); err != nil {
+		return err
+	}
+	return s.fillPendingWorkouts(ctx, limit)
+}
+
+func (s *Service) fillPendingActivityDetails(ctx context.Context, limit int) error {
 	pending, err := s.db.ActivitiesMissingDetails(ctx, s.userID, limit)
 	if err != nil {
 		return err
@@ -284,8 +315,7 @@ func (s *Service) FillPendingDetails(ctx context.Context, limit int) error {
 		return fmt.Errorf("load profile: %w", err)
 	}

-	s.setProgress(0, len(pending))
-	defer s.setProgress(0, 0)
+	s.setProgress(PhaseActivities, 0, len(pending))

 	for i, a := range pending {
 		if i > 0 {
@@ -301,7 +331,41 @@ func (s *Service) FillPendingDetails(ctx context.Context, limit int) error {
 		if err := s.ClassifyActivity(ctx, a.ID); err != nil {
 			return fmt.Errorf("classify activity %d: %w", a.GarminActivityID, err)
 		}
-		s.setProgress(i+1, len(pending))
+		s.setProgress(PhaseActivities, i+1, len(pending))
+	}
+	return nil
+}
+
+// fillPendingWorkouts fetches get_workout_by_id for up to limit activities
+// missing it. Unlike fillPendingActivityDetails, one activity's workout
+// fetch failing is non-fatal (logged, loop continues) -- workout target
+// bands are enrichment, not core activity data, and workout_raw_json
+// staying NULL means ActivitiesMissingWorkout will naturally retry it on
+// the next sync.
+func (s *Service) fillPendingWorkouts(ctx context.Context, limit int) error {
+	pending, err := s.db.ActivitiesMissingWorkout(ctx, s.userID, limit)
+	if err != nil {
+		return err
+	}
+	profile, err := s.db.GetProfile(ctx, s.userID)
+	if err != nil {
+		return fmt.Errorf("load profile: %w", err)
+	}
+
+	s.setProgress(PhaseWorkouts, 0, len(pending))
+
+	for i, a := range pending {
+		if i > 0 {
+			select {
+			case <-ctx.Done():
+				return ctx.Err()
+			case <-time.After(s.cfg.InterCallDelay):
+			}
+		}
+		if err := s.fillActivityWorkout(ctx, a, profile); err != nil {
+			log.Printf("sync: fill workout for activity %d failed, will retry next sync: %v", a.GarminActivityID, err)
+		}
+		s.setProgress(PhaseWorkouts, i+1, len(pending))
 	}
 	return nil
 }
@@ -316,23 +380,14 @@ func (s *Service) fillActivityDetails(ctx context.Context, a store.Activity, pro
 		return fmt.Errorf("get_activity_details: %w", err)
 	}

-	targets := make([]*garmin.WorkoutStep, len(splits.Laps))
-	if a.WorkoutID != nil {
-		workout, err := s.garmin.GetWorkoutByID(ctx, *a.WorkoutID)
-		if err != nil {
-			log.Printf("sync: get_workout_by_id(%d) for activity %d failed, continuing without target zones: %v", *a.WorkoutID, a.GarminActivityID, err)
-		} else {
-			targets = alignWorkoutTargets(splits.Laps, workout)
-			if err := s.db.SetActivityWorkout(ctx, s.userID, a.ID, string(workout.Raw)); err != nil {
-				return err
-			}
-		}
-	}
-
 	samples := garmin.ExtractSamples(details)
 	if err := s.db.ReplaceActivitySamples(ctx, s.userID, a.ID, toSampleRows(samples)); err != nil {
 		return err
 	}
+	// No workout-target alignment here -- that's fillActivityWorkout's job,
+	// run as its own later pass (see fillPendingWorkouts). targets is an
+	// all-nil placeholder the same length as splits.Laps.
+	targets := make([]*garmin.WorkoutStep, len(splits.Laps))
 	if err := s.db.ReplaceLaps(ctx, s.userID, a.ID, toLapRows(splits.Laps, samples, targets, profile)); err != nil {
 		return err
 	}
@@ -342,6 +397,35 @@ func (s *Service) fillActivityDetails(ctx context.Context, a store.Activity, pro
 	return s.db.SetActivitySplitsFetched(ctx, s.userID, a.ID)
 }

+// fillActivityWorkout fetches a's structured workout and re-derives its
+// laps' target pace/HR bands from it. Requires a.WorkoutID to be set --
+// only ever called for activities ActivitiesMissingWorkout returned, which
+// already filters on that. Reads laps back from the DB (already written by
+// fillActivityDetails, in some earlier pass or run) rather than needing the
+// original garmin.Lap data again, since alignWorkoutTargets only needs a
+// count.
+func (s *Service) fillActivityWorkout(ctx context.Context, a store.Activity, profile store.Profile) error {
+	workout, err := s.garmin.GetWorkoutByID(ctx, *a.WorkoutID)
+	if err != nil {
+		return fmt.Errorf("get_workout_by_id: %w", err)
+	}
+	laps, err := s.db.LapsForActivity(ctx, s.userID, a.ID)
+	if err != nil {
+		return err
+	}
+	targets := alignWorkoutTargets(len(laps), workout)
+	for i := range laps {
+		if i < len(targets) && targets[i] != nil {
+			laps[i].TargetPaceLowMps, laps[i].TargetPaceHighMps = targetPaceRange(*targets[i])
+			laps[i].TargetHRLowBpm, laps[i].TargetHRHighBpm = targetHRRange(*targets[i], profile)
+		}
+	}
+	if err := s.db.ReplaceLaps(ctx, s.userID, a.ID, laps); err != nil {
+		return err
+	}
+	return s.db.SetActivityWorkout(ctx, s.userID, a.ID, string(workout.Raw))
+}
+
 // ClassifyActivity (re)runs the rule engine for one activity against the
 // currently active workout kinds and appends a new kind_assignments row.
 // Safe to call repeatedly (e.g. after editing a workout kind's rule).