diff --git a/docs/superpowers/plans/2026-07-26-improve-synchronization-plan.md b/docs/superpowers/plans/2026-07-26-improve-synchronization-plan.md new file mode 100644 index 0000000..f51c0b4 --- /dev/null +++ b/docs/superpowers/plans/2026-07-26-improve-synchronization-plan.md @@ -0,0 +1,1743 @@ +# Improve Synchronization UX Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace the Profile page's inline "syncing: N/M activities" text with a blocking modal +that shows real, phase-aware progress through sync (discovering activities, fetching activity +details, fetching workouts), per `docs/superpowers/specs/2026-07-26-improve-synchronization-design.md`. + +**Architecture:** `sync.Service.Progress()` becomes phase-aware (`{Phase, Done, Total}` instead of +a flat `{Done, Total}`); `FillPendingDetails` splits into two sequential, independently-countable +passes (activity details, then workouts); `GET /api/sync/status` exposes the new shape; a new +frontend `SyncModal` component polls it and replaces all inline sync UI in `GarminConnection.tsx`. + +**Tech Stack:** Go (backend, `internal/sync`/`internal/store`/`internal/api`), React/TypeScript +(frontend, no test suite -- verified via `tsc -b`/`oxlint` and manual browser smoke test). + +## Global Constraints + +- Every store method takes an explicit `userID` and uses it in a real `WHERE`/`JOIN` clause (per + CLAUDE.md's per-user isolation convention) -- the two new store methods in Task 2 follow the + exact pattern of the existing `ActivitiesMissingDetails`/`CountActivitiesMissingDetails`. +- `gofmt -l .` must report nothing; `go build ./...`, `go vet ./...`, `go test ./...` must all + pass before any commit. +- Frontend: `npm run build` (`tsc -b && vite build`) and `npm run lint` (oxlint) must be clean + before any commit. No frontend test suite exists -- verify new UI manually in a browser against + `cmd/seedsample` data. +- Reset all (`ResetAll`/`handleSyncReset`) is explicitly out of scope -- do not touch it beyond + what's incidentally required (none is required). +- Follow this repo's existing JSON convention exactly: hand-built response maps use snake_case + keys (`in_progress`, `activities_pending_details`), but a Go struct serialized directly (like + `SyncRun` today) keeps its exported Go-style field names (`ID`, `Kind`, `Status`, ...) verbatim + -- the new `Progress` struct follows the same pattern (`Phase`, `Done`, `Total`, not lowercased), + consistent with how `SyncRun`/today's `DetailFillProgress` already serialize. + +--- + +### Task 1: Remove dead `Backfill`/`IncrementalSync` wrappers + +**Files:** +- Modify: `backend/internal/sync/service.go:99-123` (delete `Backfill`), `:194-209` (delete + `IncrementalSync`), `:224-232` (fix `FullSync`'s doc comment) +- Modify: `backend/internal/store/syncruns.go:9-11` (delete `SyncKindBackfill`/`SyncKindIncremental`) +- Modify: `backend/internal/sync/service_test.go` (rename/rewrite tests listed below) + +**Interfaces:** +- Consumes: nothing new. +- Produces: `backfillCore(ctx context.Context) (int, error)` and + `incrementalSyncCore(ctx context.Context) (int, error)` remain exactly as they are today (same + signatures, same package) -- every later task and every existing caller (`FullSync`) is + unaffected. `store.SyncKindFull` remains; `SyncStatusRunning`/`SyncStatusSuccess`/`SyncStatusError` + remain. + +- [ ] **Step 1: Confirm nothing outside `internal/sync` calls the exported wrappers** + +Run: `grep -rn '\.Backfill(\|\.IncrementalSync(' backend/internal/api backend/cmd` +Expected: no output (already verified during design discussion -- this is a pre-flight sanity +check, not new information). + +- [ ] **Step 2: Delete `Backfill` and `IncrementalSync`, fix `FullSync`'s doc comment** + +In `backend/internal/sync/service.go`, delete this entire method (lines 99-123): + +```go +// Backfill pages backward in Config.BackfillWindowDays windows until +// Profile.BackfillHorizonDays is reached or Garmin returns an empty page. +// The horizon is read fresh from the profile on every call (not fixed at +// server startup), so a user-edited value takes effect on the very next +// sync. Safe to re-run: activities are upserted by garmin_activity_id, and +// thanks to the sync_state watermark (Garmin history is immutable once +// recorded) a repeat call only fetches whatever's newer than the last +// completed backfill, or is a fast no-op if the configured horizon is +// already fully covered -- it does not re-walk years of already-known +// history. Widening the horizon between calls resumes further back instead +// of re-fetching everything. +func (s *Service) Backfill(ctx context.Context) error { + runID, err := s.db.StartSyncRun(ctx, s.userID, store.SyncKindBackfill) + if err != nil { + return err + } + + total, err := s.backfillCore(ctx) + if err != nil { + msg := err.Error() + s.db.FinishSyncRun(ctx, s.userID, runID, total, &msg) + return err + } + return s.db.FinishSyncRun(ctx, s.userID, runID, total, nil) +} +``` + +Replace the comment on `backfillCore` (still present right after, just above `func (s *Service) +backfillCore`) with: + +```go +// backfillCore pages backward in Config.BackfillWindowDays windows until +// Profile.BackfillHorizonDays is reached or Garmin returns an empty page. +// The horizon is read fresh from the profile on every call (not fixed at +// server startup), so a user-edited value takes effect on the very next +// sync. Safe to re-run: activities are upserted by garmin_activity_id, and +// thanks to the sync_state watermark (Garmin history is immutable once +// recorded) a repeat call only fetches whatever's newer than the last +// completed backfill, or is a fast no-op if the configured horizon is +// already fully covered -- it does not re-walk years of already-known +// history. Widening the horizon between calls resumes further back instead +// of re-fetching everything. Used by FullSync as one step of its single +// combined SyncRun; there is no standalone entrypoint for this anymore +// (the periodic background sync loop that used to call one is gone -- see +// 4d2cbe4 refactor: remove automatic background incremental sync). +func (s *Service) backfillCore(ctx context.Context) (int, error) { +``` + +Delete this entire method (lines 194-209 in the original): + +```go +// IncrementalSync fetches activities from just before the latest known +// activity (or a short recent window if none exist yet) through today. +func (s *Service) IncrementalSync(ctx context.Context) error { + runID, err := s.db.StartSyncRun(ctx, s.userID, store.SyncKindIncremental) + if err != nil { + return err + } + + n, err := s.incrementalSyncCore(ctx) + if err != nil { + msg := err.Error() + s.db.FinishSyncRun(ctx, s.userID, runID, n, &msg) + return err + } + return s.db.FinishSyncRun(ctx, s.userID, runID, n, nil) +} +``` + +Replace `incrementalSyncCore`'s comment (unchanged signature, just the doc): + +```go +// incrementalSyncCore fetches activities from just before the latest known +// activity (or a short recent window if none exist yet) through today. Used +// by FullSync as one step of its single combined SyncRun -- see +// backfillCore's comment for why there's no standalone entrypoint. +func (s *Service) incrementalSyncCore(ctx context.Context) (int, error) { +``` + +Replace `FullSync`'s doc comment: + +```go +// FullSync performs a complete manual "Sync now" pass -- backfillCore +// (resumes from the watermark), then incrementalSyncCore (catches anything +// new since the latest known activity), then FillPendingDetails -- +// recorded as a single SyncRun so the reported activity count covers the +// whole action instead of only whichever stage happened to finish last. +func (s *Service) FullSync(ctx context.Context, detailFillLimit int) error { +``` + +- [ ] **Step 3: Remove the now-dead sync-kind constants** + +In `backend/internal/store/syncruns.go`, change: + +```go +const ( + SyncKindBackfill = "backfill" + SyncKindIncremental = "incremental" + // SyncKindFull is a manually-triggered "Sync now" pass: Backfill followed + // by IncrementalSync followed by FillPendingDetails, recorded as one run + // so the reported activity count covers the whole action instead of only + // whichever stage happened to finish last. + SyncKindFull = "full" + + SyncStatusRunning = "running" + SyncStatusSuccess = "success" + SyncStatusError = "error" +) +``` + +to: + +```go +const ( + // SyncKindFull is a manually-triggered "Sync now" pass: backfillCore + // followed by incrementalSyncCore followed by FillPendingDetails, + // recorded as one run so the reported activity count covers the whole + // action instead of only whichever stage happened to finish last. + SyncKindFull = "full" + + SyncStatusRunning = "running" + SyncStatusSuccess = "success" + SyncStatusError = "error" +) +``` + +- [ ] **Step 4: Update `backend/internal/sync/service_test.go`'s callers** + +Rename and rewrite (drop the redundant SyncRun assertion -- `TestFullSync_RecordsOneCombinedSyncRun` +already covers SyncRun recording thoroughly): + +```go +func TestBackfillCore_StoresActivities(t *testing.T) { + db := openTestDB(t) + ctx := context.Background() + userID := provisionTestUser(t, db) + + m := &mock.Client{Activities: []garmin.Activity{ + {ActivityID: 1, ActivityName: "Morning Run", ActivityType: garmin.ActivityType{TypeKey: "running"}, + StartTimeGMT: "2026-07-01 06:00:00", Distance: 5000, Duration: 1500, AverageSpeed: 3.33, AverageHR: 145}, + }} + svc := NewService(m, db, userID, Config{}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) + + total, err := svc.backfillCore(ctx) + if err != nil { + t.Fatalf("backfillCore: %v", err) + } + if total != 1 { + t.Errorf("backfillCore returned total = %d, want 1", total) + } + + activities, err := db.ListActivities(ctx, userID, store.ActivityFilter{}) + if err != nil { + t.Fatalf("ListActivities: %v", err) + } + if len(activities) != 1 { + t.Fatalf("expected 1 stored activity, got %d", len(activities)) + } + if activities[0].GarminActivityID != 1 { + t.Errorf("GarminActivityID = %d, want 1", activities[0].GarminActivityID) + } +} +``` + +Rename `TestBackfill_SkipsNonRunningActivities` to `TestBackfillCore_SkipsNonRunningActivities`, +replacing its body's `if err := svc.Backfill(ctx); err != nil {` with +`if _, err := svc.backfillCore(ctx); err != nil {` (message text `"Backfill: %v"` -> +`"backfillCore: %v"`). + +Rename `TestBackfill_SecondRunIsANoOpOnceHorizonFullyCovered` to +`TestBackfillCore_SecondRunIsANoOpOnceHorizonFullyCovered`, replacing both +`if err := svc.Backfill(ctx); err != nil {` occurrences with +`if _, err := svc.backfillCore(ctx); err != nil {` (messages `"first Backfill: %v"` -> +`"first backfillCore: %v"`, `"second Backfill: %v"` -> `"second backfillCore: %v"`). + +Rename `TestBackfill_ResumesFromWatermarkWhenHorizonGrows` to +`TestBackfillCore_ResumesFromWatermarkWhenHorizonGrows`, same replacement pattern (`"first +Backfill: %v"` -> `"first backfillCore: %v"`, `"second Backfill: %v"` -> `"second backfillCore: +%v"`). + +In the remaining tests, keep the test name unchanged but swap the call (message text follows the +same `"Backfill: %v"` -> `"backfillCore: %v"` pattern) in: +`TestFillPendingDetailsAndClassify_EndToEnd`, `TestFillPendingDetails_ResolvesWorkoutTargetsOntoLaps`, +`TestFillPendingDetails_OneExtraTrailingLapKeepsOtherLapsTargets`, +`TestResetAll_AllowsFreshBackfillAfterwards` (both `svc.Backfill(ctx)` calls -- messages `"first +Backfill: %v"` -> `"first backfillCore: %v"`, `"Backfill after reset: %v"` -> `"backfillCore +after reset: %v"`), `TestService_TwoUsersSyncIndependently` (both `svcA.Backfill(ctx)` and +`svcB.Backfill(ctx)`, messages `"Backfill(a): %v"` -> `"backfillCore(a): %v"`, `"Backfill(b): %v"` +-> `"backfillCore(b): %v"`). + +Leave `TestFillPendingDetails_ReportsLiveProgress` untouched here -- it's fully rewritten in Task 3. + +- [ ] **Step 5: Run the full backend test suite** + +Run: `cd backend && go build ./... && go vet ./... && gofmt -l . && go test ./...` +Expected: all packages pass, `gofmt -l .` prints nothing. + +- [ ] **Step 6: Commit** + +```bash +git add backend/internal/sync/service.go backend/internal/sync/service_test.go backend/internal/store/syncruns.go +git commit -m "$(cat <<'EOF' +refactor(sync): remove dead Backfill/IncrementalSync exported wrappers + +Both were only reachable via the periodic background sync loop removed +in 4d2cbe4 -- nothing in production calls them anymore, only tests did. +FullSync already calls backfillCore/incrementalSyncCore directly. +Rewrite the affected tests to call the *Core functions (still exported +within the package) instead, dropping the now-redundant standalone +SyncRun-recording assertion covered by TestFullSync_RecordsOneCombinedSyncRun. +EOF +)" +``` + +--- + +### Task 2: Add `ActivitiesMissingWorkout`/`CountActivitiesMissingWorkout` store methods + +**Files:** +- Modify: `backend/internal/store/activities.go:263-274` (add two new methods right after + `CountActivitiesMissingDetails`) +- Modify: `backend/internal/store/store_test.go` (add a new test) + +**Interfaces:** +- Consumes: `activityColumns` constant, `scanActivity` helper, `Activity` struct (all already + exist in `activities.go`). +- Produces: `db.ActivitiesMissingWorkout(ctx, userID, limit) ([]Activity, error)` and + `db.CountActivitiesMissingWorkout(ctx, userID) (int, error)` -- Task 4 (`FillPendingDetails` + split) calls both. + +- [ ] **Step 1: Write the failing test** + +Add to `backend/internal/store/store_test.go`: + +```go +func TestActivitiesMissingWorkout_IndependentOfDetailsFetchStatus(t *testing.T) { + db := openTestDB(t) + ctx := context.Background() + userID, err := db.ProvisionUser(ctx, "test-sub", "Test") + if err != nil { + t.Fatalf("ProvisionUser: %v", err) + } + + workoutID := int64(999) + withWorkoutNoDetails := Activity{ + GarminActivityID: 1, WorkoutID: &workoutID, + StartTimeUTC: "2026-07-01 06:00:00", RawJSON: "{}", + } + idA, err := db.UpsertActivity(ctx, userID, withWorkoutNoDetails) + if err != nil { + t.Fatalf("UpsertActivity (a): %v", err) + } + + withWorkoutAndDetails := Activity{ + GarminActivityID: 2, WorkoutID: &workoutID, + StartTimeUTC: "2026-07-02 06:00:00", RawJSON: "{}", + } + idB, err := db.UpsertActivity(ctx, userID, withWorkoutAndDetails) + if err != nil { + t.Fatalf("UpsertActivity (b): %v", err) + } + // Simulate an activity whose details/splits were already fetched in a + // prior run, but whose workout fetch failed back then -- this is exactly + // the case ActivitiesMissingWorkout must still surface, since it's + // queried independently of details_fetched_at/splits_fetched_at. + if err := db.SetActivityDetails(ctx, userID, idB, "{}"); err != nil { + t.Fatalf("SetActivityDetails (b): %v", err) + } + if err := db.SetActivitySplitsFetched(ctx, userID, idB); err != nil { + t.Fatalf("SetActivitySplitsFetched (b): %v", err) + } + + noWorkout := Activity{ + GarminActivityID: 3, StartTimeUTC: "2026-07-03 06:00:00", RawJSON: "{}", + } + if _, err := db.UpsertActivity(ctx, userID, noWorkout); err != nil { + t.Fatalf("UpsertActivity (c): %v", err) + } + + alreadyHasWorkout := Activity{ + GarminActivityID: 4, WorkoutID: &workoutID, + StartTimeUTC: "2026-07-04 06:00:00", RawJSON: "{}", + } + idD, err := db.UpsertActivity(ctx, userID, alreadyHasWorkout) + if err != nil { + t.Fatalf("UpsertActivity (d): %v", err) + } + if err := db.SetActivityWorkout(ctx, userID, idD, `{"segments":[]}`); err != nil { + t.Fatalf("SetActivityWorkout (d): %v", err) + } + + n, err := db.CountActivitiesMissingWorkout(ctx, userID) + if err != nil { + t.Fatalf("CountActivitiesMissingWorkout: %v", err) + } + if n != 2 { + t.Fatalf("CountActivitiesMissingWorkout = %d, want 2 (activities a and b)", n) + } + + pending, err := db.ActivitiesMissingWorkout(ctx, userID, 10) + if err != nil { + t.Fatalf("ActivitiesMissingWorkout: %v", err) + } + if len(pending) != 2 { + t.Fatalf("ActivitiesMissingWorkout returned %d activities, want 2", len(pending)) + } + ids := map[int64]bool{} + for _, a := range pending { + ids[a.ID] = true + } + if !ids[idA] || !ids[idB] { + t.Errorf("ActivitiesMissingWorkout = %+v, want to include activities a and b", pending) + } +} +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `cd backend && go test ./internal/store/... -run TestActivitiesMissingWorkout -v` +Expected: FAIL with `db.CountActivitiesMissingWorkout undefined` / `db.ActivitiesMissingWorkout +undefined`. + +- [ ] **Step 3: Implement the two store methods** + +In `backend/internal/store/activities.go`, right after `CountActivitiesMissingDetails` (after +line 274), add: + +```go + +// ActivitiesMissingWorkout returns userID's activities that have a +// structured workout (WorkoutID set at initial upsert time, straight from +// Garmin's activity summary) but haven't had get_workout_by_id fetched yet. +// Independently queryable from ActivitiesMissingDetails: workout_id is +// known well before any detail fetch, and workout_raw_json is only ever +// set by SetActivityWorkout, so this also picks up an activity whose +// details were fetched successfully in some prior run but whose workout +// fetch failed back then -- ActivitiesMissingDetails would never surface +// that activity again (details_fetched_at/splits_fetched_at are already +// set), silently losing its target pace/HR bands forever without this. +func (db *DB) ActivitiesMissingWorkout(ctx context.Context, userID int64, limit int) ([]Activity, error) { + rows, err := db.QueryContext(ctx, `SELECT `+activityColumns+` FROM activities + WHERE user_id = ? AND workout_id IS NOT NULL AND workout_raw_json IS NULL + ORDER BY start_time_utc DESC LIMIT ?`, userID, limit) + if err != nil { + return nil, fmt.Errorf("list activities missing workout for user %d: %w", userID, err) + } + defer rows.Close() + + activities := []Activity{} + for rows.Next() { + a, err := scanActivity(rows) + if err != nil { + return nil, fmt.Errorf("scan activity row: %w", err) + } + activities = append(activities, a) + } + return activities, rows.Err() +} + +// CountActivitiesMissingWorkout returns how many of userID's activities +// still need get_workout_by_id fetched, regardless of any per-call batch +// limit -- used to report overall remaining work, mirroring +// CountActivitiesMissingDetails. +func (db *DB) CountActivitiesMissingWorkout(ctx context.Context, userID int64) (int, error) { + var n int + err := db.QueryRowContext(ctx, `SELECT COUNT(*) FROM activities + WHERE user_id = ? AND workout_id IS NOT NULL AND workout_raw_json IS NULL`, userID).Scan(&n) + if err != nil { + return 0, fmt.Errorf("count activities missing workout for user %d: %w", userID, err) + } + return n, nil +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `cd backend && go test ./internal/store/... -run TestActivitiesMissingWorkout -v` +Expected: PASS + +- [ ] **Step 5: Run the full store package test suite** + +Run: `cd backend && go test ./internal/store/... && gofmt -l internal/store/` +Expected: all pass, `gofmt` prints nothing. + +- [ ] **Step 6: Commit** + +```bash +git add backend/internal/store/activities.go backend/internal/store/store_test.go +git commit -m "$(cat <<'EOF' +feat(store): add ActivitiesMissingWorkout/CountActivitiesMissingWorkout + +Mirrors ActivitiesMissingDetails/CountActivitiesMissingDetails, but +queries workout_id IS NOT NULL AND workout_raw_json IS NULL -- +independent of whether details/splits were ever fetched, since +workout_id is known at initial upsert time. This also fixes a latent +gap: an activity whose details were fetched successfully but whose +workout fetch failed in the same run previously had no way to ever be +retried, since ActivitiesMissingDetails stops returning it the moment +details_fetched_at/splits_fetched_at are set. +EOF +)" +``` + +--- + +### Task 3: Split `FillPendingDetails` into activities/workouts phases with phase-aware `Progress` + +**Files:** +- Modify: `backend/internal/sync/service.go:57-97` (Progress type + phase constants, + `setProgress`), `:233-261` (`FullSync`, add discovering-phase progress), `:271-343` + (`FillPendingDetails` split into two functions, `fillActivityDetails` simplified, new + `fillActivityWorkout`) +- Modify: `backend/internal/sync/mapping.go:113-125` (`alignWorkoutTargets` signature) +- Modify: `backend/internal/garmin/mock/mock.go` (add `WorkoutErrByID`) +- Modify: `backend/internal/sync/service_test.go` (update `alignWorkoutTargets` call sites, + rewrite `TestFillPendingDetails_ReportsLiveProgress`, add new tests) + +**Interfaces:** +- Consumes: `db.ActivitiesMissingWorkout`/`db.CountActivitiesMissingWorkout` (Task 2), + `db.LapsForActivity`/`db.ReplaceLaps`/`db.SetActivityWorkout` (already exist). +- Produces: `Progress{Phase, Done, Total}` with `Phase` one of `PhaseIdle`, `PhaseDiscovering`, + `PhaseActivities`, `PhaseWorkouts` (all exported `string` constants) -- Task 4 (`/api/sync/status` + handler) reads `svc.Progress()` and serializes it as-is. + +- [ ] **Step 1: Update `alignWorkoutTargets`'s existing tests to the new signature first** + +`alignWorkoutTargets` only ever uses `len(laps)`, never any lap's actual content -- change its +signature to take the count directly, since Task 3's `fillActivityWorkout` (added in Step 4) +needs to call it without holding a `[]garmin.Lap` (it only has `[]store.Lap` read back from the +DB). In `backend/internal/sync/service_test.go`, update all three call sites: + +```go +targets := alignWorkoutTargets(len(laps), workout) +``` + +(replacing `alignWorkoutTargets(laps, workout)` in `TestAlignWorkoutTargets_MatchesLapsToFlattenedSteps`, +`TestAlignWorkoutTargets_OneExtraTrailingLapKeepsOtherTargets`, and +`TestAlignWorkoutTargets_MismatchedCountYieldsAllNil` -- the `laps := []garmin.Lap{...}` setup +lines stay exactly as they are, only the call itself changes). + +- [ ] **Step 2: Run to verify it fails** + +Run: `cd backend && go test ./internal/sync/... -run TestAlignWorkoutTargets -v` +Expected: FAIL with a compile error (`alignWorkoutTargets(laps []garmin.Lap, ...)` still expects +a `[]garmin.Lap`, not an `int` -- `len(laps)` is an `int`, mismatched argument type). + +- [ ] **Step 3: Change `alignWorkoutTargets`'s signature** + +In `backend/internal/sync/mapping.go`, replace: + +```go +func alignWorkoutTargets(laps []garmin.Lap, workout garmin.Workout) []*garmin.WorkoutStep { + steps := workout.FlattenSteps() + out := make([]*garmin.WorkoutStep, len(laps)) + + switch len(laps) - len(steps) { + case 0, 1: + for i := range steps { + s := steps[i] + out[i] = &s + } + } + return out +} +``` + +with: + +```go +func alignWorkoutTargets(lapCount int, workout garmin.Workout) []*garmin.WorkoutStep { + steps := workout.FlattenSteps() + out := make([]*garmin.WorkoutStep, lapCount) + + switch lapCount - len(steps) { + case 0, 1: + for i := range steps { + s := steps[i] + out[i] = &s + } + } + return out +} +``` + +(Also update the doc comment above it, which still says "zips an activity's recorded laps +against its structured workout's flattened steps" -- change the first line to "zips an activity's +lap count against its structured workout's flattened steps", the rest of the comment about the +one-extra-trailing-lap case stays accurate as-is.) + +- [ ] **Step 4: Run to verify it passes** + +Run: `cd backend && go test ./internal/sync/... -run TestAlignWorkoutTargets -v` +Expected: PASS (note: `fillActivityDetails` in `service.go` still calls +`alignWorkoutTargets(splits.Laps, workout)` at this point -- that call site is rewritten in Step 6 +below, so the package won't fully build until then; run this test with `-run +TestAlignWorkoutTargets` specifically, or expect a build failure from the not-yet-updated call +site if running the whole package). + +- [ ] **Step 5: Add phase-aware `Progress` and update `FullSync`** + +In `backend/internal/sync/service.go`, replace: + +```go +// Progress reports how far a currently-running (or just-finished) +// FillPendingDetails pass has gotten, for a status banner to poll. +type Progress struct { + Done int + Total int +} +``` + +with: + +```go +// 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 +} +``` + +Replace `setProgress`: + +```go +func (s *Service) setProgress(done, total int) { + s.progressMu.Lock() + s.progress = Progress{Done: done, Total: total} + s.progressMu.Unlock() +} +``` + +with: + +```go +func (s *Service) setProgress(phase string, done, total int) { + s.progressMu.Lock() + s.progress = Progress{Phase: phase, Done: done, Total: total} + s.progressMu.Unlock() +} +``` + +Also update the doc comment right above `Progress()`'s accessor method (unchanged signature): + +```go +// Progress returns the current sync progress (phase idle, 0/0 when nothing +// is running). +func (s *Service) Progress() Progress { +``` + +In `FullSync`, add phase tracking around the discovering stage. Replace: + +```go +func (s *Service) FullSync(ctx context.Context, detailFillLimit int) error { + runID, err := s.db.StartSyncRun(ctx, s.userID, store.SyncKindFull) + if err != nil { + return err + } + + backfillCount, err := s.backfillCore(ctx) +``` + +with: + +```go +func (s *Service) FullSync(ctx context.Context, detailFillLimit int) error { + runID, err := s.db.StartSyncRun(ctx, s.userID, store.SyncKindFull) + 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) +``` + +(the rest of `FullSync`'s body -- the `incrementalSyncCore` call, the `FillPendingDetails` call, +`FinishSyncRun` -- stays exactly as it is; the `defer` above covers every return path). + +- [ ] **Step 6: Split `FillPendingDetails` into two phases; simplify `fillActivityDetails`; add `fillActivityWorkout`** + +Replace the entire block from `FillPendingDetails` through the end of `fillActivityDetails` +(currently lines 309-379: `FillPendingDetails` and `fillActivityDetails`) with: + +```go +// 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 + } + profile, err := s.db.GetProfile(ctx, s.userID) + if err != nil { + return fmt.Errorf("load profile: %w", err) + } + + s.setProgress(PhaseActivities, 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.fillActivityDetails(ctx, a, profile); err != nil { + return fmt.Errorf("fill details for activity %d: %w", a.GarminActivityID, err) + } + if err := s.ClassifyActivity(ctx, a.ID); err != nil { + return fmt.Errorf("classify activity %d: %w", a.GarminActivityID, err) + } + 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 +} + +func (s *Service) fillActivityDetails(ctx context.Context, a store.Activity, profile store.Profile) error { + splits, err := s.garmin.GetActivitySplits(ctx, a.GarminActivityID) + if err != nil { + return fmt.Errorf("get_activity_splits: %w", err) + } + details, err := s.garmin.GetActivityDetails(ctx, a.GarminActivityID) + if err != nil { + return fmt.Errorf("get_activity_details: %w", 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 + } + if err := s.db.SetActivityDetails(ctx, s.userID, a.ID, string(details.Raw)); err != nil { + return err + } + 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)) +} +``` + +- [ ] **Step 7: Add `WorkoutErrByID` to the mock client** + +In `backend/internal/garmin/mock/mock.go`, add a field to `Client`: + +```go +type Client struct { + AuthResults []garmin.AuthResult // consumed in order by Authenticate/CompleteMFA calls + Activities []garmin.Activity + Splits map[int64]garmin.ActivitySplits + Details map[int64]garmin.ActivityDetails + Workouts map[int64]garmin.Workout + // WorkoutErrByID, if set for a given workout ID, makes GetWorkoutByID + // return that error for that ID specifically -- independent of the + // all-calls-fail Err field below -- so a test can simulate one + // activity's workout fetch failing while others in the same batch + // succeed. + WorkoutErrByID map[int64]error + Err error // if set, every call returns this error + authResultCursor int + ClosedCalled bool + GetActivitiesCalls int + AuthenticateCalls int + LastEmail string + LastPassword string +} +``` + +Update `GetWorkoutByID`: + +```go +func (c *Client) GetWorkoutByID(ctx context.Context, workoutID int64) (garmin.Workout, error) { + if c.Err != nil { + return garmin.Workout{}, c.Err + } + if err, ok := c.WorkoutErrByID[workoutID]; ok { + return garmin.Workout{}, err + } + return c.Workouts[workoutID], nil +} +``` + +- [ ] **Step 8: Rewrite `TestFillPendingDetails_ReportsLiveProgress` for phase-awareness, add two new tests** + +Replace `TestFillPendingDetails_ReportsLiveProgress` in `backend/internal/sync/service_test.go` +with: + +```go +func TestFillPendingDetails_ReportsPhaseAwareProgressThroughActivitiesAndWorkoutsPhases(t *testing.T) { + db := openTestDB(t) + ctx := context.Background() + userID := provisionTestUser(t, db) + + const n = 2 + m := &mock.Client{ + Activities: []garmin.Activity{}, + Splits: map[int64]garmin.ActivitySplits{}, + Details: map[int64]garmin.ActivityDetails{}, + Workouts: map[int64]garmin.Workout{}, + } + for i := int64(1); i <= n; i++ { + workoutID := i + 100 + m.Activities = append(m.Activities, garmin.Activity{ + ActivityID: i, ActivityType: garmin.ActivityType{TypeKey: "running"}, WorkoutID: &workoutID, + StartTimeGMT: "2026-07-0" + string(rune('0'+i)) + " 06:00:00", Distance: 5000, Duration: 1500, + }) + m.Splits[i] = garmin.ActivitySplits{ActivityID: i} + m.Details[i] = garmin.ActivityDetails{ActivityID: i} + m.Workouts[workoutID] = garmin.Workout{} + } + + svc := NewService(m, db, userID, Config{InterCallDelay: 150 * time.Millisecond}, + fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) + if _, err := svc.backfillCore(ctx); err != nil { + t.Fatalf("backfillCore: %v", err) + } + + if p := svc.Progress(); p.Phase != PhaseIdle { + t.Fatalf("Progress before FillPendingDetails = %+v, want PhaseIdle", p) + } + + done := make(chan error, 1) + go func() { done <- svc.FillPendingDetails(ctx, n) }() + + // Timeline with InterCallDelay=150ms and n=2 per phase: activities pass + // does item0 (instant), delays ~150ms, does item1 -- landing here around + // t=75ms should be mid-delay in the activities phase. + time.Sleep(75 * time.Millisecond) + midActivities := svc.Progress() + if midActivities.Phase != PhaseActivities { + t.Errorf("mid-activities Progress().Phase = %q, want %q", midActivities.Phase, PhaseActivities) + } + if midActivities.Total != n { + t.Errorf("mid-activities Progress().Total = %d, want %d", midActivities.Total, n) + } + if midActivities.Done != 1 { + t.Errorf("mid-activities Progress().Done = %d, want 1", midActivities.Done) + } + + // The activities phase finishes its own delay+item1 around t=150ms, then + // the workouts phase starts: item0 (instant), delay ~150ms, item1. + // Landing around t=150+75=225ms should be mid-delay in the workouts phase. + time.Sleep(150 * time.Millisecond) + midWorkouts := svc.Progress() + if midWorkouts.Phase != PhaseWorkouts { + t.Errorf("mid-workouts Progress().Phase = %q, want %q", midWorkouts.Phase, PhaseWorkouts) + } + if midWorkouts.Total != n { + t.Errorf("mid-workouts Progress().Total = %d, want %d", midWorkouts.Total, n) + } + if midWorkouts.Done != 1 { + t.Errorf("mid-workouts Progress().Done = %d, want 1", midWorkouts.Done) + } + + if err := <-done; err != nil { + t.Fatalf("FillPendingDetails: %v", err) + } + if final := svc.Progress(); final.Phase != PhaseIdle || final.Total != 0 || final.Done != 0 { + t.Errorf("Progress after completion = %+v, want zero-value PhaseIdle", final) + } +} + +func TestFillPendingDetails_RetriesWorkoutFetchForActivityWithDetailsAlreadyFetched(t *testing.T) { + db := openTestDB(t) + ctx := context.Background() + userID := provisionTestUser(t, db) + + const garminActivityID = 77 + const workoutID = 888 + workoutIDPtr := int64(workoutID) + id, err := db.UpsertActivity(ctx, userID, store.Activity{ + GarminActivityID: garminActivityID, WorkoutID: &workoutIDPtr, + StartTimeUTC: "2026-07-01 06:00:00", RawJSON: "{}", + }) + if err != nil { + t.Fatalf("UpsertActivity: %v", err) + } + // Simulate a prior run that successfully fetched details/splits but + // whose workout fetch failed (workout_raw_json stays NULL). + if err := db.SetActivityDetails(ctx, userID, id, "{}"); err != nil { + t.Fatalf("SetActivityDetails: %v", err) + } + if err := db.SetActivitySplitsFetched(ctx, userID, id); err != nil { + t.Fatalf("SetActivitySplitsFetched: %v", err) + } + if err := db.ReplaceLaps(ctx, userID, id, []store.Lap{{LapIndex: 1, IntensityType: "ACTIVE"}}); err != nil { + t.Fatalf("ReplaceLaps: %v", err) + } + + m := &mock.Client{ + Workouts: map[int64]garmin.Workout{ + workoutID: {Segments: []garmin.WorkoutSegment{ + {Steps: []garmin.WorkoutStep{ + {Type: "ExecutableStepDTO", TargetType: garmin.WorkoutTargetType{TypeKey: "pace.zone"}, TargetValueOne: f(3.0), TargetValueTwo: f(3.5)}, + }}, + }}, + }, + } + svc := NewService(m, db, userID, Config{}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) + + // A prior FillPendingDetails call (which had ActivitiesMissingDetails + // return nothing, since details are already fetched) still reaches this + // activity via ActivitiesMissingWorkout. + if err := svc.FillPendingDetails(ctx, 10); err != nil { + t.Fatalf("FillPendingDetails: %v", err) + } + + laps, err := db.LapsForActivity(ctx, userID, id) + if err != nil || len(laps) != 1 { + t.Fatalf("LapsForActivity: %v, %+v", err, laps) + } + if laps[0].TargetPaceLowMps == nil || *laps[0].TargetPaceLowMps != 3.0 { + t.Errorf("TargetPaceLowMps = %v, want 3.0 (workout fetch should have been retried)", laps[0].TargetPaceLowMps) + } +} + +func TestFillPendingDetails_OneWorkoutFetchFailureDoesNotAbortTheWorkoutsPass(t *testing.T) { + db := openTestDB(t) + ctx := context.Background() + userID := provisionTestUser(t, db) + + const failingWorkoutID = 200 + const okWorkoutID = 201 + failingPtr, okPtr := int64(failingWorkoutID), int64(okWorkoutID) + m := &mock.Client{ + Activities: []garmin.Activity{ + {ActivityID: 1, ActivityType: garmin.ActivityType{TypeKey: "running"}, WorkoutID: &failingPtr, + StartTimeGMT: "2026-07-01 06:00:00", Distance: 5000, Duration: 1500}, + {ActivityID: 2, ActivityType: garmin.ActivityType{TypeKey: "running"}, WorkoutID: &okPtr, + StartTimeGMT: "2026-07-02 06:00:00", Distance: 5000, Duration: 1500}, + }, + Splits: map[int64]garmin.ActivitySplits{ + 1: {ActivityID: 1, Laps: []garmin.Lap{{LapIndex: 1, IntensityType: "ACTIVE"}}}, + 2: {ActivityID: 2, Laps: []garmin.Lap{{LapIndex: 1, IntensityType: "ACTIVE"}}}, + }, + Details: map[int64]garmin.ActivityDetails{1: {ActivityID: 1}, 2: {ActivityID: 2}}, + Workouts: map[int64]garmin.Workout{ + okWorkoutID: {Segments: []garmin.WorkoutSegment{ + {Steps: []garmin.WorkoutStep{ + {Type: "ExecutableStepDTO", TargetType: garmin.WorkoutTargetType{TypeKey: "pace.zone"}, TargetValueOne: f(3.0), TargetValueTwo: f(3.5)}, + }}, + }}, + }, + WorkoutErrByID: map[int64]error{failingWorkoutID: fmt.Errorf("garmin says no")}, + } + // Not timing-sensitive -- keep InterCallDelay negligible so the 2 + // activities/2 workouts here don't cost real wall-clock seconds + // (Config{}'s default is 1s per gap). + svc := NewService(m, db, userID, Config{InterCallDelay: time.Millisecond}, + fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) + + if _, err := svc.backfillCore(ctx); err != nil { + t.Fatalf("backfillCore: %v", err) + } + if err := svc.FillPendingDetails(ctx, 10); err != nil { + t.Fatalf("FillPendingDetails should not return an error for a per-activity workout fetch failure: %v", err) + } + + remaining, err := db.CountActivitiesMissingWorkout(ctx, userID) + if err != nil { + t.Fatalf("CountActivitiesMissingWorkout: %v", err) + } + if remaining != 1 { + t.Errorf("CountActivitiesMissingWorkout = %d, want 1 (the failing activity stays pending, retryable next sync)", remaining) + } + + activities, err := db.ListActivities(ctx, userID, store.ActivityFilter{}) + if err != nil { + t.Fatalf("ListActivities: %v", err) + } + var okActivity store.Activity + for _, a := range activities { + if a.GarminActivityID == 2 { + okActivity = a + } + } + laps, err := db.LapsForActivity(ctx, userID, okActivity.ID) + if err != nil || len(laps) != 1 { + t.Fatalf("LapsForActivity: %v, %+v", err, laps) + } + if laps[0].TargetPaceLowMps == nil || *laps[0].TargetPaceLowMps != 3.0 { + t.Errorf("the succeeding activity's TargetPaceLowMps = %v, want 3.0 (should not be affected by the other activity's failure)", laps[0].TargetPaceLowMps) + } +} +``` + +`fmt` is already imported in `service_test.go`'s package (`package sync` imports it transitively +via other test helpers)? Check: if `go vet`/`go build` complains `fmt` is undefined in +`service_test.go`, add `"fmt"` to that file's import block. + +- [ ] **Step 9: Run the full `internal/sync` test suite** + +Run: `cd backend && go test ./internal/sync/... -v 2>&1 | tail -80` +Expected: every test passes, including the three new/rewritten ones above and every existing +test renamed/adjusted in Task 1. + +- [ ] **Step 10: Run the full backend suite** + +Run: `cd backend && go build ./... && go vet ./... && gofmt -l . && go test ./...` +Expected: all pass, `gofmt -l .` prints nothing. + +- [ ] **Step 11: Commit** + +```bash +git add backend/internal/sync/service.go backend/internal/sync/service_test.go backend/internal/sync/mapping.go backend/internal/garmin/mock/mock.go +git commit -m "$(cat <<'EOF' +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. +EOF +)" +``` + +--- + +### Task 4: Reshape `GET /api/sync/status` and update frontend types + +**Files:** +- Modify: `backend/internal/api/sync.go:69-102` (`handleSyncStatus`) +- Modify: `backend/internal/api/api_test.go` (add a new test) +- Modify: `frontend/src/types/api.ts:197-207` (`DetailFillProgress`/`SyncStatus`, `SyncRun.Kind`) + +**Interfaces:** +- Consumes: `svc.Progress()` (Task 3), `db.CountActivitiesMissingWorkout` (Task 2). +- Produces: `GET /api/sync/status` response shape + `{in_progress, progress: {Phase, Done, Total}, activities_pending_details, workouts_pending, + last_run?}` -- Task 5/6 (frontend) consume this exact shape. + +- [ ] **Step 1: Write the failing backend test** + +Add to `backend/internal/api/api_test.go`: + +```go +func TestSyncStatus_ReportsPhaseAwareProgressAndWorkoutsPending(t *testing.T) { + s, db, userID := newTestServer(t) + ctx := newCtx() + + workoutID := int64(555) + if _, err := db.UpsertActivity(ctx, userID, store.Activity{ + GarminActivityID: 1, WorkoutID: &workoutID, + StartTimeUTC: "2026-07-11 06:00:00", RawJSON: "{}", + }); err != nil { + t.Fatalf("UpsertActivity: %v", err) + } + + rec := doJSON(t, s.Router(), http.MethodGet, "/api/sync/status", nil) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, body = %s", rec.Code, rec.Body.String()) + } + var resp struct { + InProgress bool `json:"in_progress"` + Progress struct { + Phase string `json:"Phase"` + Done int `json:"Done"` + Total int `json:"Total"` + } `json:"progress"` + ActivitiesPendingDetails int `json:"activities_pending_details"` + WorkoutsPending int `json:"workouts_pending"` + } + unmarshalBody(t, rec, &resp) + + if resp.InProgress { + t.Error("in_progress = true, want false (nothing running)") + } + if resp.Progress.Phase != "idle" { + t.Errorf("progress.Phase = %q, want %q", resp.Progress.Phase, "idle") + } + if resp.ActivitiesPendingDetails != 1 { + t.Errorf("activities_pending_details = %d, want 1", resp.ActivitiesPendingDetails) + } + if resp.WorkoutsPending != 1 { + t.Errorf("workouts_pending = %d, want 1", resp.WorkoutsPending) + } +} +``` + +If `unmarshalBody` isn't already available in `api_test.go` (it's defined in `setup_test.go` per +earlier work this session), no import change is needed -- both files are in `package api`. + +- [ ] **Step 2: Run it to verify it fails** + +Run: `cd backend && go test ./internal/api/... -run TestSyncStatus_ReportsPhaseAware -v` +Expected: FAIL -- `resp.WorkoutsPending`/`resp.Progress.Phase` come back zero-valued because the +handler doesn't produce `workouts_pending` or a nested `progress` object yet (still +`detail_fill_progress`). + +- [ ] **Step 3: Update the handler** + +In `backend/internal/api/sync.go`, replace `handleSyncStatus`: + +```go +func (s *Server) handleSyncStatus(w http.ResponseWriter, r *http.Request) { + userID := userIDFromContext(r.Context()) + run, ok, err := s.DB.LatestSyncRun(r.Context(), userID) + if err != nil { + writeError(w, http.StatusInternalServerError, err.Error()) + return + } + pendingDetails, err := s.DB.CountActivitiesMissingDetails(r.Context(), userID) + if err != nil { + writeError(w, http.StatusInternalServerError, err.Error()) + return + } + pendingWorkouts, err := s.DB.CountActivitiesMissingWorkout(r.Context(), userID) + if err != nil { + writeError(w, http.StatusInternalServerError, err.Error()) + return + } + + s.mu.Lock() + inProgress := s.userSyncRunning[userID] + s.mu.Unlock() + + svc, err := s.syncFor(r.Context(), userID) + if err != nil { + writeError(w, http.StatusInternalServerError, err.Error()) + return + } + progress := svc.Progress() + + resp := map[string]any{ + "in_progress": inProgress, + "progress": progress, + "activities_pending_details": pendingDetails, + "workouts_pending": pendingWorkouts, + } + if ok { + resp["last_run"] = run + } + writeJSON(w, http.StatusOK, resp) +} +``` + +- [ ] **Step 4: Run to verify it passes** + +Run: `cd backend && go test ./internal/api/... -run TestSyncStatus_ReportsPhaseAware -v` +Expected: PASS + +- [ ] **Step 5: Run the full backend suite** + +Run: `cd backend && go build ./... && go vet ./... && gofmt -l . && go test ./...` +Expected: all pass, `gofmt -l .` prints nothing. + +- [ ] **Step 6: Update the frontend types** + +In `frontend/src/types/api.ts`, replace: + +```ts +export interface SyncRun { + ID: number; + Kind: "backfill" | "incremental"; + StartedAt: string; + FinishedAt: string | null; + ActivitiesFetched: number; + Status: "running" | "success" | "error"; + ErrorMessage: string | null; +} +``` + +with (adding the previously-missing `"full"` -- production has only ever recorded `"full"` since +the background sync loop was removed, but `"backfill"`/`"incremental"` remain valid for +historical rows already in a real database): + +```ts +export interface SyncRun { + ID: number; + Kind: "backfill" | "incremental" | "full"; + StartedAt: string; + FinishedAt: string | null; + ActivitiesFetched: number; + Status: "running" | "success" | "error"; + ErrorMessage: string | null; +} +``` + +Replace: + +```ts +export interface DetailFillProgress { + Done: number; + Total: number; +} + +export interface SyncStatus { + in_progress: boolean; + detail_fill_progress: DetailFillProgress; + activities_pending_details: number; + last_run?: SyncRun; +} +``` + +with: + +```ts +export interface SyncProgress { + Phase: "idle" | "discovering" | "activities" | "workouts"; + Done: number; + Total: number; +} + +export interface SyncStatus { + in_progress: boolean; + progress: SyncProgress; + activities_pending_details: number; + workouts_pending: number; + last_run?: SyncRun; +} +``` + +- [ ] **Step 7: Verify the frontend still type-checks** + +Run: `cd frontend && npm run build 2>&1 | tail -30` +Expected: FAILS at this point -- `GarminConnection.tsx` still references +`status.detail_fill_progress`/`DetailFillProgress`, which no longer exist. This is expected and +resolved in Task 6; do not attempt to fix `GarminConnection.tsx` in this task. + +- [ ] **Step 8: Commit** + +```bash +git add backend/internal/api/sync.go backend/internal/api/api_test.go frontend/src/types/api.ts +git commit -m "$(cat <<'EOF' +feat(api): reshape /api/sync/status for phase-aware progress + +Replaces detail_fill_progress ({Done, Total}) with progress ({Phase, +Done, Total}), and adds workouts_pending (mirroring +activities_pending_details) from the new CountActivitiesMissingWorkout. +Frontend types updated to match -- GarminConnection.tsx is intentionally +left broken by this commit alone; it's fixed in the next commit that +adds SyncModal. +EOF +)" +``` + +--- + +### Task 5: Add `SyncModal` component + +**Files:** +- Create: `frontend/src/components/SyncModal.tsx` +- Modify: `frontend/src/App.css` (add sync-modal-specific styles, reusing the existing + `.modal-backdrop`/`.modal-content`/`.modal-header` classes from `RawDataModal`) + +**Interfaces:** +- Consumes: `api.syncStatus()` (`frontend/src/api/client.ts`, unchanged signature -- only its + resolved `SyncStatus` shape changed, in Task 4), `SyncStatus`/`SyncProgress` types (Task 4). +- Produces: `SyncModal({ onClose: () => void })` component -- Task 6 renders it from + `GarminConnection.tsx`. + +- [ ] **Step 1: Add sync-modal CSS** + +In `frontend/src/App.css`, right after the existing `.modal-json` block (after line 486), add: + +```css +.sync-modal-content { + width: min(420px, 90vw); +} + +.sync-modal-body { + padding: 1.5rem 1rem; + display: flex; + flex-direction: column; + align-items: center; + gap: 0.75rem; + text-align: center; +} + +.sync-modal-progress { + width: 100%; + height: 0.6rem; +} + +.sync-modal-label { + margin: 0; + color: #9aa0ab; + font-size: 0.9rem; +} + +.sync-modal-spinner { + width: 2rem; + height: 2rem; + border: 3px solid #2a2d35; + border-top-color: #3b82f6; + border-radius: 50%; + animation: sync-modal-spin 0.8s linear infinite; +} + +@keyframes sync-modal-spin { + to { + transform: rotate(360deg); + } +} + +.sync-modal-actions { + justify-content: flex-end; + padding: 0.75rem 1rem; + border-top: 1px solid #2a2d35; +} +``` + +- [ ] **Step 2: Create `SyncModal.tsx`** + +```tsx +import { useEffect, useRef, useState } from "react"; +import { api } from "../api/client"; +import type { SyncStatus } from "../types/api"; + +function phaseDisplay(status: SyncStatus): { label: string; bar: { done: number; total: number } | null } { + const { Phase, Done, Total } = status.progress; + switch (Phase) { + case "discovering": + return { label: "Discovering activities…", bar: null }; + case "activities": + return { label: `Activities: ${Done}/${Total}`, bar: { done: Done, total: Total } }; + case "workouts": + return { label: `Workouts: ${Done}/${Total}`, bar: { done: Done, total: Total } }; + default: + return { label: "Starting…", bar: null }; + } +} + +// Blocking overlay shown while "Sync now" is running -- the only place sync +// progress/results are shown (see docs/superpowers/specs/2026-07-26-improve-synchronization-design.md). +// Never auto-closes, even on success: the user decides when to dismiss it, +// so a sync error can't be missed by looking away for a moment. +export function SyncModal({ onClose }: { onClose: () => void }) { + const [status, setStatus] = useState(null); + const [error, setError] = useState(null); + const stoppedRef = useRef(false); + + useEffect(() => { + stoppedRef.current = false; + let timeout: ReturnType; + const tick = () => { + api + .syncStatus() + .then((s) => { + setStatus(s); + setError(null); + }) + .catch((e) => setError(String(e))) + .finally(() => { + if (!stoppedRef.current) timeout = setTimeout(tick, 1500); + }); + }; + tick(); + return () => { + stoppedRef.current = true; + clearTimeout(timeout); + }; + }, []); + + const inProgress = status?.in_progress ?? true; + const { label, bar } = status ? phaseDisplay(status) : { label: "Starting…", bar: null }; + + return ( +
+
+
+ Synchronizing +
+
+ {inProgress ? ( + bar ? ( + <> + +

{label}

+ + ) : ( + <> +
+

{label}

+ + ) + ) : ( + <> + {status?.last_run && ( +

+ {status.last_run.Status === "error" + ? `Sync failed: ${status.last_run.ErrorMessage}` + : `Sync complete: ${status.last_run.ActivitiesFetched} new activities`} +

+ )} + {status && status.activities_pending_details > 0 && ( +

+ {status.activities_pending_details} more activities pending — click Sync now again +

+ )} + {status && status.workouts_pending > 0 && ( +

+ {status.workouts_pending} more workouts pending — click Sync now again +

+ )} + + )} + {error &&

{error}

} +
+
+ +
+
+
+ ); +} +``` + +- [ ] **Step 3: Verify it builds and lints in isolation** + +Run: `cd frontend && npm run build 2>&1 | tail -30` +Expected: still FAILS, but only on `GarminConnection.tsx` (`SyncModal.tsx` itself introduces no +new errors -- it isn't imported/used by anything yet, so `tsc` type-checks it standalone +successfully; confirm the only reported errors are in `GarminConnection.tsx`). + +Run: `cd frontend && npm run lint 2>&1 | tail -30` +Expected: no new warnings/errors attributable to `SyncModal.tsx`. + +- [ ] **Step 4: Commit** + +```bash +git add frontend/src/components/SyncModal.tsx frontend/src/App.css +git commit -m "$(cat <<'EOF' +feat(frontend): add SyncModal component + +Blocking overlay polling /api/sync/status, rendering by progress.Phase +(indeterminate spinner while discovering, a real progress bar for the +activities/workouts phases), then the final sync result (success count, +or the error message that was already fetched but never rendered +before) plus the existing pending-details/pending-workouts nudge. Never +auto-closes -- not yet wired into GarminConnection.tsx (next commit). +EOF +)" +``` + +--- + +### Task 6: Wire `SyncModal` into `GarminConnection.tsx` + +**Files:** +- Modify: `frontend/src/components/GarminConnection.tsx` + +**Interfaces:** +- Consumes: `SyncModal` (Task 5). +- Produces: nothing further consumes `GarminConnection.tsx` -- this is the final integration + point for this feature. + +- [ ] **Step 1: Replace the whole file** + +`GarminConnection.tsx` keeps its connect/MFA/Reset-all logic completely unchanged. It loses: +`syncStatus` state, `syncStatusRef`, the `syncProgressLabel` helper, `syncStatus` polling inside +`refreshStatus`/the mount `useEffect`, and all inline sync-progress/last-sync JSX. It gains a +`showSyncModal` boolean and renders `` when true. + +```tsx +import { useEffect, useState } from "react"; +import { api } from "../api/client"; +import type { AuthResponse } from "../types/api"; +import { SyncModal } from "./SyncModal"; + +export function GarminConnection({ onBeforeConnect }: { onBeforeConnect?: () => Promise }) { + const [auth, setAuth] = useState(null); + const [code, setCode] = useState(""); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(null); + // There's no real Garmin logout -- the backend session just sits idle. + // "Disconnect" only hides that and shows Connect again locally; a real + // login (connect()) clears it. + const [disconnected, setDisconnected] = useState(false); + const [showSyncModal, setShowSyncModal] = useState(false); + + // Auth status alone -- sync status is polled by SyncModal itself while + // it's open, so this no longer needs the "poll faster while syncing" + // dynamic interval it used to have. + useEffect(() => { + api.authStatus().then(setAuth).catch((e) => setError(String(e))); + const interval = setInterval(() => { + api.authStatus().then(setAuth).catch(() => {}); + }, 6000); + return () => clearInterval(interval); + }, []); + + async function connect() { + setBusy(true); + setError(null); + try { + await onBeforeConnect?.(); + setAuth(await api.login()); + setDisconnected(false); + } catch (e) { + setError(String(e)); + } finally { + setBusy(false); + } + } + + function disconnect() { + setDisconnected(true); + } + + async function submitMFA() { + if (!code.trim()) return; + setBusy(true); + setError(null); + try { + setAuth(await api.submitMFA(code.trim())); + setCode(""); + } catch (e) { + setError(String(e)); + } finally { + setBusy(false); + } + } + + async function sync() { + setBusy(true); + setError(null); + try { + await api.syncRun(); + setShowSyncModal(true); + } catch (e) { + setError(String(e)); + } finally { + setBusy(false); + } + } + + async function resetAll() { + if (!window.confirm("This deletes every synced activity, lap, and kind assignment, then starts a fresh pull from Garmin next sync. This cannot be undone. Continue?")) { + return; + } + setBusy(true); + setError(null); + try { + await api.resetSync(); + // Reset runs as a background sync job; wait for it to actually finish + // before reloading, otherwise other pages (e.g. Activities) would + // still show the just-deleted activities from their own stale state. + let status = await api.syncStatus(); + while (status.in_progress) { + await new Promise((resolve) => setTimeout(resolve, 300)); + status = await api.syncStatus(); + } + window.location.reload(); + } catch (e) { + setError(String(e)); + setBusy(false); + } + } + + const status = disconnected ? "unknown" : auth?.status ?? "unknown"; + + return ( +
+
+
+ {status !== "authenticated" && status !== "mfa_required" && ( + + )} + + {status === "authenticated" && ( + <> + + + + )} + + {/* Reset all only wipes local DB state (see handleSyncReset) -- it + doesn't touch the Garmin session, so it stays available even + while disconnected/not-yet-connected. */} + +
+ +
+ + {/* Connected/not-connected is already conveyed by the + Connect/Disconnect button itself -- only MFA/failed need a + label, since no button distinguishes those from "not connected". */} + {(status === "mfa_required" || status === "failed") && ( + + {status === "mfa_required" ? "MFA code required" : "Connection failed"} + + )} +
+
+ + {status === "mfa_required" && ( +
+ setCode(e.target.value)} + onKeyDown={(e) => e.key === "Enter" && submitMFA()} + /> + +
+ )} + + {!disconnected && auth?.message &&

{auth.message}

} + + {error &&

{error}

} + + {showSyncModal && setShowSyncModal(false)} />} +
+ ); +} +``` + +- [ ] **Step 2: Verify the frontend builds and lints cleanly** + +Run: `cd frontend && npm run build 2>&1 | tail -30` +Expected: clean build, no errors. + +Run: `cd frontend && npm run lint 2>&1 | tail -30` +Expected: only the pre-existing `PaceField.tsx` fast-refresh warnings (unrelated to this change). + +- [ ] **Step 3: Manual smoke test** + +Start both dev servers against `cmd/seedsample` data (or a real Garmin-connected profile), open +the Profile page, click "Sync now", and confirm: +- The modal appears immediately, showing a spinner + "Discovering activities…". +- It transitions to a progress bar labeled "Activities: N/M", then "Workouts: N/M" (if any + activities have a `workout_id`). +- Once finished, it shows the final result (success + activity count, or the error message) and + a "Close" button, and does **not** auto-close. +- Clicking "Close" hides the modal; the Profile page no longer shows any inline sync text at all + (neither live progress nor a static "last sync" line). +- Clicking "Sync now" again while already open a second time is prevented by the existing 409 + "a sync is already in progress" (surfaced as the existing inline `error` paragraph). + +- [ ] **Step 4: Commit** + +```bash +git add frontend/src/components/GarminConnection.tsx +git commit -m "$(cat <<'EOF' +feat(frontend): replace inline sync progress with SyncModal + +GarminConnection.tsx no longer polls sync status itself or renders any +inline "syncing.../last sync..." text -- SyncModal (previous commit) is +now the only place sync progress and results are shown. Auth-status +polling is simplified to a fixed interval now that it no longer needs +to speed up while a sync is running. +EOF +)" +``` + +--- + +## Final verification + +- [ ] Run the full backend suite one more time: `cd backend && go build ./... && go vet ./... && gofmt -l . && go test ./...` +- [ ] Run the full frontend build/lint one more time: `cd frontend && npm run build && npm run lint` +- [ ] Re-run the manual smoke test from Task 6, Step 3, end to end. +- [ ] Use superpowers:finishing-a-development-branch to wrap up (tests green -> present the + merge/PR/keep-as-is menu).