Files
geniusrun/docs/superpowers/specs/2026-07-26-improve-synchronization-design.md

150 lines
8.0 KiB
Markdown
Raw Normal View History

# 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 `service.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
`FillPendingDetails``Backfill`/`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
```go
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. **`activities`** — `FillPendingDetails` 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:
```json
{
"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 `syncStatus` 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.