Files
geniusrun/docs/superpowers/specs/2026-07-26-improve-synchronization-design.md
Christophe Vila e2b2bf9611 refactor: merge internal/sync into internal/garmin, regroup api files and routes
Garmin auth/sync routes move under /api/garmin/*; sync.Service becomes
garmin.Sync with garmin.SyncConfig/ClientConfig; applog becomes
internal/log; the test mock moves into the garmin package as MockClient
(breaking the test-only import cycle the merge created); stale test
URLs and type names updated to match.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-04 16:04:18 +02:00

8.0 KiB

Improve Synchronization UX — Design

Source idea: docs/IDEAS.md — "improve synchronization: make it modal, split downloads of activities and workouts (as workouts are currently downloaded from their ID in related activities), add progress bar (which requires to know in advance how many activities or workouts will have to be downloaded)."

Goal

Replace the Profile page's single inline "syncing: N/M activities" text line with a blocking modal that shows real progress through sync, broken into named phases the user can actually follow: discovering new activities, fetching activity details, then fetching workouts.

Cleanup folded into this work: dead Backfill/IncrementalSync wrappers

Service.Backfill(ctx) and Service.IncrementalSync(ctx) are exported methods that each record their own SyncRun, wrapping backfillCore/incrementalSyncCore. Their doc comments say they're used "by the periodic background loop" -- but that loop was removed in an earlier session (4d2cbe4 refactor: remove automatic background incremental sync), and grepping internal/api/ and cmd/ confirms nothing in production calls them anymore. FullSync (the only production caller of the core logic) calls backfillCore/incrementalSyncCore directly. Only internal/sync/service_test.go still calls Backfill/IncrementalSync, which is the only reason they're not already flagged as unused by the compiler.

Since this plan already restructures sync.go's progress model, remove these two dead exported methods (and their now-inaccurate doc comments) as an early task, rewriting the tests that called them to exercise the same behavior through FullSync or the *Core functions directly (same package, so unexported functions are still directly testable) -- before any of the progress-model changes below, so later tasks aren't touching code that's about to be deleted.

Current state (for reference)

  • sync.Service.Progress() returns a flat {Done, Total}, written only by FillPendingDetailsBackfill/IncrementalSync (the "discover which activities exist" phase) report no progress at all today.
  • FillPendingDetails fetches, per activity, in one pass: GetActivitySplits, GetActivityDetails, and (if the activity has a workout_id) GetWorkoutByID — then writes samples, laps (with workout-derived target pace/HR bands baked in), activity details, and marks splits fetched.
  • GET /api/sync/status returns {in_progress, detail_fill_progress: {Done, Total}, activities_pending_details, last_run?}. The frontend (GarminConnection.tsx) polls this and renders an inline <p> line, both while syncing and once idle (a static "last sync: ..." summary).
  • sync_runs stores one row per Backfill/IncrementalSync/FullSync call (kind/started_at/finished_at/activities_fetched/status/error_message). GET /api/sync/runs exists but nothing in the frontend calls it.

Scope

In scope: "Sync now" (FullSync) gets the new modal and phase-aware progress. The static idle "last sync" summary on the Profile page is removed — the modal becomes the only place sync status/results are shown; closing it means nothing is shown again until the next sync.

Out of scope: "Reset all" keeps its current behavior unchanged (confirm dialog, fire, poll via a blocking while loop, reload the page) — no modal, no progress UI. The "modern error displays" idea (ephemeral banners, backend-down handling) from the same backlog section is a separate, later feature — this one only makes sync's own errors visible via the modal's final state, reusing the sync_runs.error_message field that's already fetched but never rendered today.

Backend design

Phase-aware progress

type Progress struct {
    Phase string // "idle" | "discovering" | "activities" | "workouts"
    Done  int
    Total int
}

FullSync drives the phase transitions:

  1. discovering — set (Done=0, Total=0, indeterminate) while Backfill/IncrementalSync run. These still report no count; the modal shows a spinner, not a bar, during this phase.
  2. activitiesFillPendingDetails is split into two sequential passes. The first pass fetches GetActivitySplits+GetActivityDetails+samples for every activity missing details (ActivitiesMissingDetails/CountActivitiesMissingDetails, unchanged queries), writing laps without workout-target alignment yet. Total is the pending-details count taken once at the start of this pass; Done advances per activity.
  3. workouts — a new second pass. workout_id is decoded from Garmin's activity summary and stored on the activities row at upsert time, well before any detail fetch — so "activities needing a workout fetched" is an independently queryable condition (workout_id IS NOT NULL AND workout_raw_json IS NULL), needing no live Garmin call to count. Two new store methods, ActivitiesMissingWorkout/CountActivitiesMissingWorkout (mirroring the naming of the existing ActivitiesMissingDetails/ CountActivitiesMissingDetails), list/count these. For each: fetch GetWorkoutByID, re-derive lap target bands from the just-stored lap data (alignWorkoutTargets), update the laps and set workout_raw_json. This condition also picks up any older activity that has a workout_id but never got its workout aligned in some prior run (e.g. a past transient failure) — not just ones touched in this run's activities pass — a small latent-bug fix as a side effect.
  4. idle — reset to {Phase: "idle", Done: 0, Total: 0} once FullSync returns, same defer-based reset as today.

Restructuring fillActivityDetails to decouple lap-writing from workout-target alignment means an activity with a workout gets its laps written twice (once plain in the activities pass, once updated with target bands in the workouts pass) — this is a second local SQL delete+insert, not a second Garmin API call, so it costs nothing against rate limits.

API

GET /api/sync/status changes shape:

{
  "in_progress": true,
  "progress": { "phase": "workouts", "done": 3, "total": 8 },
  "activities_pending_details": 0,
  "last_run": { "...": "SyncRun, unchanged fields" }
}

This replaces detail_fill_progress/DetailFillProgress outright (no other consumer exists). SyncRun.Kind's frontend type also gains its missing "full" value (backend has always been able to report it; the frontend type was just never updated to match).

Frontend design

GarminConnection.tsx keeps its connect/MFA/Reset-all logic and buttons untouched, but loses its garminSyncStatus polling, syncProgressLabel helper, and all inline sync-progress/last-sync JSX.

A new SyncModal.tsx:

  • Renders as a blocking overlay (page behind it non-interactive) the moment api.syncRun() resolves successfully.
  • Polls api.syncStatus() every 1500ms while open (same cadence as today's polling).
  • Renders by progress.phase:
    • discovering → spinner + "Discovering activities…"
    • activities → progress bar + "Activities: {done}/{total}"
    • workouts → progress bar + "Workouts: {done}/{total}"
  • Once in_progress becomes false: shows last_run.Status, ActivitiesFetched, and ErrorMessage (if the run errored) plus the existing "N more pending — sync again" nudge if activities_pending_details > 0, and a Close button. No auto-close, no timeout — the user decides when to dismiss it (this is the one place sync errors are visible, so nothing should hide it automatically).

Testing

  • internal/sync: new/updated tests for the two-phase FillPendingDetails split (an activity with a workout gets its laps written in both passes, target bands only present after the workouts pass; an activity without a workout is untouched by the workouts pass), and for phase-aware Progress() transitions.
  • internal/api: sync_test.go updated for the new /api/sync/status JSON shape.
  • Frontend: no test suite exists (per CLAUDE.md) — manual smoke test against seedsample data, watching the modal move through all four phases.