From b5cd47c219dc70fde1cbc8cdae480a95b18165ab Mon Sep 17 00:00:00 2001 From: Christophe Vila Date: Mon, 27 Jul 2026 18:45:11 +0200 Subject: [PATCH] docs: add implementation plan for permanently-missing-workout handling 4 tasks: mark a wrapper-level 404 as not_found, expose it as garmin.ErrNotFound in the Go client, add a workout_not_found_at column + store query updates, then have fillPendingWorkouts stop retrying a confirmed-404 workout instead of retrying forever. Co-Authored-By: Claude Sonnet 5 --- .../2026-07-27-workout-not-found-plan.md | 844 ++++++++++++++++++ 1 file changed, 844 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-27-workout-not-found-plan.md diff --git a/docs/superpowers/plans/2026-07-27-workout-not-found-plan.md b/docs/superpowers/plans/2026-07-27-workout-not-found-plan.md new file mode 100644 index 0000000..c48b40d --- /dev/null +++ b/docs/superpowers/plans/2026-07-27-workout-not-found-plan.md @@ -0,0 +1,844 @@ +# Stop Retrying a Permanently-Missing Garmin Workout 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:** Stop `fillPendingWorkouts` from retrying a `get_workout_by_id` call forever when Garmin +returns a definitive 404 (the workout was deleted after being linked to an activity), per +`docs/superpowers/specs/2026-07-27-workout-not-found-design.md`. + +**Architecture:** `garminconnect`'s own `GarminConnectNotFoundError` (raised for any HTTP 404) is +caught specifically in `wrapper.py`'s dispatch loop and marked with `"not_found": true` in the +JSON error response. `internal/garmin/client.go` turns that into a Go sentinel error +(`ErrNotFound`) any caller can `errors.Is` against. `internal/sync/service.go`'s +`fillPendingWorkouts` checks for it and, only for that specific case, calls a new store method +(`SetActivityWorkoutNotFound`) that permanently excludes the activity from +`ActivitiesMissingWorkout`/`CountActivitiesMissingWorkout` -- `workout_raw_json` stays `NULL` +forever (no fabricated data), but a new `workout_not_found_at` column distinguishes "confirmed +missing" from "not yet fetched." + +**Tech Stack:** Go (backend, `internal/garmin`/`internal/store`/`internal/sync`), Python +(`internal/garmin/pyscript/wrapper.py`). No frontend changes. + +## Global Constraints + +- `gofmt -l .` must report nothing; `go build ./...`, `go vet ./...`, `go test ./...` must all + pass before any commit. +- Every store method takes an explicit `userID` and uses it in a real `WHERE`/`JOIN` clause (per + this repo's per-user isolation convention) -- `SetActivityWorkoutNotFound` follows the exact + pattern of the existing `SetActivitySplitsFetched`. +- No migration history: `internal/store/schema.sql` is edited directly, then + `go run ./cmd/dumpschema` regenerates `docs/DATABASE.md`. +- `fillActivityDetails`/`fillPendingActivityDetails`'s error handling (abort-the-batch-and-report) + is explicitly out of scope -- this fix only changes `fillPendingWorkouts`'s behavior. +- No frontend changes -- `workouts_pending` simply stops counting a confirmed-missing workout. + +--- + +### Task 1: `wrapper.py` marks a 404 with `not_found: true` + +**Files:** +- Modify: `backend/internal/garmin/pyscript/wrapper.py` (imports, `dispatch`) +- Modify: `backend/internal/garmin/pyscript/tests/test_wrapper.py` (new test) + +**Interfaces:** +- Consumes: `garminconnect.GarminConnectNotFoundError` (already installed as a dependency). +- Produces: `dispatch()`'s returned error dict gains an additional `"not_found": true` key + whenever the caught exception is (or is a subclass of) `GarminConnectNotFoundError` -- Task 2 + reads this field on the Go side. + +- [ ] **Step 1: Write the failing test** + +Add to `backend/internal/garmin/pyscript/tests/test_wrapper.py`: + +```python +def test_call_marks_not_found_error_specifically(): + from garminconnect import GarminConnectNotFoundError + + wrapper._auth_state = "authenticated" + wrapper._client = MagicMock() + wrapper._client.get_workout_by_id.side_effect = GarminConnectNotFoundError("API Error 404") + resp = wrapper.dispatch({ + "id": 11, "cmd": "call", "params": {"method": "get_workout_by_id", "args": {"workout_id": "999"}}, + }) + assert resp == {"id": 11, "error": "API Error 404", "not_found": True} + + +def test_call_does_not_mark_other_errors_as_not_found(): + wrapper._auth_state = "authenticated" + wrapper._client = MagicMock() + wrapper._client.get_workout_by_id.side_effect = Exception("rate limited") + resp = wrapper.dispatch({ + "id": 12, "cmd": "call", "params": {"method": "get_workout_by_id", "args": {"workout_id": "999"}}, + }) + assert resp == {"id": 12, "error": "rate limited"} +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `cd backend/internal/garmin/pyscript && python3 -m pytest tests/test_wrapper.py -k not_found -v` +(use whatever Python interpreter this repo's wrapper tests normally run under -- see +`backend/internal/garmin/pyscript/.venv` if one exists, or the `GARMIN_WRAPPER_PYTHON` convention). +Expected: `test_call_marks_not_found_error_specifically` FAILS (`resp` has no `"not_found"` key +yet); `test_call_does_not_mark_other_errors_as_not_found` already passes (nothing to change for +that case). + +- [ ] **Step 3: Update `dispatch()`** + +In `backend/internal/garmin/pyscript/wrapper.py`, replace: + +```python +from garminconnect import Garmin +``` + +with: + +```python +from garminconnect import Garmin, GarminConnectNotFoundError +``` + +Replace: + +```python +def dispatch(req): + handler = _HANDLERS.get(req.get("cmd")) + if handler is None: + return {"id": req.get("id"), "error": f"unknown cmd {req.get('cmd')!r}"} + try: + result = handler(req.get("params") or {}) + return {"id": req["id"], "result": result} + except Exception as exc: + _debug(f"{req.get('cmd')} raised {type(exc).__name__}: {exc}") + _debug(traceback.format_exc()) + return {"id": req.get("id"), "error": str(exc)} +``` + +with: + +```python +def dispatch(req): + handler = _HANDLERS.get(req.get("cmd")) + if handler is None: + return {"id": req.get("id"), "error": f"unknown cmd {req.get('cmd')!r}"} + try: + result = handler(req.get("params") or {}) + return {"id": req["id"], "result": result} + except Exception as exc: + _debug(f"{req.get('cmd')} raised {type(exc).__name__}: {exc}") + _debug(traceback.format_exc()) + resp = {"id": req.get("id"), "error": str(exc)} + # A 404 (e.g. get_workout_by_id for a workout deleted on Garmin's + # side after being linked to an activity) is definitive, not a + # transient failure worth retrying forever -- marked specifically so + # internal/garmin/client.go can tell the two apart (see + # docs/superpowers/specs/2026-07-27-workout-not-found-design.md). + if isinstance(exc, GarminConnectNotFoundError): + resp["not_found"] = True + return resp +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `cd backend/internal/garmin/pyscript && python3 -m pytest tests/test_wrapper.py -v` +Expected: all tests PASS, including both new ones and every existing test unchanged. + +- [ ] **Step 5: Commit** + +```bash +git add backend/internal/garmin/pyscript/wrapper.py backend/internal/garmin/pyscript/tests/test_wrapper.py +git commit -m "$(cat <<'EOF' +feat(garmin): mark a wrapper 404 with not_found in the error response + +garminconnect's own GarminConnectNotFoundError already exists +specifically for this (its docstring: "so callers can now catch a +missing resource specifically, e.g. deleting an already-deleted +workout"), raised by connectapi() for any real HTTP 404. dispatch() +now surfaces that distinction as an extra not_found: true field +alongside the existing error string, so internal/garmin/client.go +(next commit) can tell a definitive 404 apart from a transient +failure. +EOF +)" +``` + +--- + +### Task 2: `internal/garmin/client.go` exposes `ErrNotFound` + +**Files:** +- Modify: `backend/internal/garmin/client.go` (`wireResponse`, new `ErrNotFound`, `roundTrip`) +- Modify: `backend/internal/garmin/client_test.go` (`wireResponsePayload`/harness, new test) + +**Interfaces:** +- Consumes: `wireResponse.NotFound` (wire field written by Task 1's `wrapper.py` change). +- Produces: `var ErrNotFound error` -- Task 4 (`fillPendingWorkouts`) checks + `errors.Is(err, garmin.ErrNotFound)` against errors returned by `GetWorkoutByID` (and, since + this is wired at the generic `roundTrip` level, any other `Client` method too). + +- [ ] **Step 1: Write the failing test** + +Add to `backend/internal/garmin/client_test.go`, right after `fakeError`: + +```go +func fakeNotFoundError(msg string) wireResponsePayload { + return wireResponsePayload{err: msg, notFound: true} +} +``` + +Update the `wireResponsePayload` struct (near the top of the file) from: + +```go +type wireResponsePayload struct { + result json.RawMessage + err string +} +``` + +to: + +```go +type wireResponsePayload struct { + result json.RawMessage + err string + notFound bool +} +``` + +Update `newFakeWrapperClient`'s harness -- replace: + +```go + payload := handle(req.Cmd, req.Params) + resp := wireResponse{ID: req.ID, Result: payload.result, Error: payload.err} +``` + +with: + +```go + payload := handle(req.Cmd, req.Params) + resp := wireResponse{ID: req.ID, Result: payload.result, Error: payload.err, NotFound: payload.notFound} +``` + +Add a new test, right after `TestSubprocessClient_RoundTrip_WrapperErrorPropagates`: + +```go +func TestSubprocessClient_RoundTrip_NotFoundWrapsErrNotFound(t *testing.T) { + c := newFakeWrapperClient(t, func(cmd string, params json.RawMessage) wireResponsePayload { + return fakeNotFoundError("API Error 404") + }) + + _, err := c.roundTrip(context.Background(), "call", nil) + if !errors.Is(err, ErrNotFound) { + t.Fatalf("roundTrip error = %v, want errors.Is(err, ErrNotFound)", err) + } + if !strings.Contains(err.Error(), "API Error 404") { + t.Errorf("roundTrip error = %v, want it to still contain the original message", err) + } +} + +func TestSubprocessClient_RoundTrip_OrdinaryErrorDoesNotWrapErrNotFound(t *testing.T) { + c := newFakeWrapperClient(t, func(cmd string, params json.RawMessage) wireResponsePayload { + return fakeError("boom") + }) + + _, err := c.roundTrip(context.Background(), "call", nil) + if errors.Is(err, ErrNotFound) { + t.Fatalf("roundTrip error = %v, want errors.Is(err, ErrNotFound) to be false", err) + } +} +``` + +Add `"errors"` to this file's import block (alongside the existing `"strings"` etc.). + +- [ ] **Step 2: Run it to verify it fails** + +Run: `cd backend && go test ./internal/garmin/... -run TestSubprocessClient_RoundTrip_NotFound -v` +Expected: FAIL to compile (`wireResponsePayload` has no field `notFound` yet -- wait, Step 1 above +already added it to the test file; the actual compile failure is `wireResponse` has no field +`NotFound` yet, and `ErrNotFound` is undefined) -- confirms the test exercises code that doesn't +exist yet. + +- [ ] **Step 3: Add `ErrNotFound` and wire it through `roundTrip`** + +In `backend/internal/garmin/client.go`, replace: + +```go +// wireResponse is one line read from the wrapper subprocess's stdout. +type wireResponse struct { + ID int `json:"id"` + Result json.RawMessage `json:"result,omitempty"` + Error string `json:"error,omitempty"` +} +``` + +with: + +```go +// wireResponse is one line read from the wrapper subprocess's stdout. +type wireResponse struct { + ID int `json:"id"` + Result json.RawMessage `json:"result,omitempty"` + Error string `json:"error,omitempty"` + NotFound bool `json:"not_found,omitempty"` +} + +// ErrNotFound wraps any error a Client method returns when the wrapper +// reported a definitive HTTP 404 (garminconnect's own +// GarminConnectNotFoundError) -- e.g. GetWorkoutByID for a workout deleted +// on Garmin's side after being linked to an activity. Callers use +// errors.Is(err, ErrNotFound) to distinguish this from a transient failure +// worth retrying. +var ErrNotFound = errors.New("garmin: resource not found") +``` + +Replace, in `roundTrip`: + +```go + if resp.Error != "" { + err = fmt.Errorf("%s: %s", cmdName, resp.Error) + return nil, err + } +``` + +with: + +```go + if resp.Error != "" { + if resp.NotFound { + err = fmt.Errorf("%s: %s: %w", cmdName, resp.Error, ErrNotFound) + } else { + err = fmt.Errorf("%s: %s", cmdName, resp.Error) + } + return nil, err + } +``` + +Add `"errors"` to `client.go`'s import block. + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `cd backend && go test ./internal/garmin/... -v` +Expected: all tests PASS, including the two new ones and every existing test unchanged. + +- [ ] **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: Commit** + +```bash +git add backend/internal/garmin/client.go backend/internal/garmin/client_test.go +git commit -m "$(cat <<'EOF' +feat(garmin): expose ErrNotFound for a wrapper-reported 404 + +roundTrip now wraps the returned error with the new ErrNotFound +sentinel whenever the wrapper's response set not_found (Task 1), +letting any Client method's caller distinguish a definitive 404 from +a transient failure via errors.Is, regardless of which garminconnect +method was called. +EOF +)" +``` + +--- + +### Task 3: Store gains `workout_not_found_at` + +**Files:** +- Modify: `backend/internal/store/schema.sql` (`activities` table) +- Modify: `backend/internal/store/activities.go` (`Activity` struct, `activityColumns`, + `scanActivity`, new `SetActivityWorkoutNotFound`, `ActivitiesMissingWorkout`/ + `CountActivitiesMissingWorkout`) +- Modify: `backend/internal/store/store_test.go` (extend the existing test, add a new one) +- Regenerate: `docs/DATABASE.md` (via `go run ./cmd/dumpschema`) + +**Interfaces:** +- Consumes: nothing new. +- Produces: `db.SetActivityWorkoutNotFound(ctx, userID, activityID int64) error` -- Task 4 + (`fillPendingWorkouts`) calls this. `Activity.WorkoutNotFoundAt *string` -- available to any + caller of `GetActivity`/`ListActivities`/etc. that wants to check it (none currently do, besides + this task's own test). + +- [ ] **Step 1: Write the failing test** + +Add to `backend/internal/store/store_test.go`, a new test (don't modify the existing +`TestActivitiesMissingWorkout_OnlyIncludesActivitiesWithDetailsAlreadyFetched` -- this is +additive): + +```go +func TestActivitiesMissingWorkout_ExcludesConfirmedNotFound(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) + // Details already fetched, workout confirmed 404 on Garmin -- must not + // be retried, so must not appear in ActivitiesMissingWorkout/Count. + notFound := Activity{ + GarminActivityID: 1, WorkoutID: &workoutID, + StartTimeUTC: "2026-07-01 06:00:00", RawJSON: "{}", + } + idA, err := db.UpsertActivity(ctx, userID, notFound) + if err != nil { + t.Fatalf("UpsertActivity (a): %v", err) + } + if err := db.SetActivityDetails(ctx, userID, idA, "{}"); err != nil { + t.Fatalf("SetActivityDetails (a): %v", err) + } + if err := db.SetActivitySplitsFetched(ctx, userID, idA); err != nil { + t.Fatalf("SetActivitySplitsFetched (a): %v", err) + } + if err := db.SetActivityWorkoutNotFound(ctx, userID, idA); err != nil { + t.Fatalf("SetActivityWorkoutNotFound (a): %v", err) + } + + // A genuinely still-pending activity (details fetched, workout not yet + // attempted) must still be included, for contrast. + stillPending := Activity{ + GarminActivityID: 2, WorkoutID: &workoutID, + StartTimeUTC: "2026-07-02 06:00:00", RawJSON: "{}", + } + idB, err := db.UpsertActivity(ctx, userID, stillPending) + if err != nil { + t.Fatalf("UpsertActivity (b): %v", err) + } + 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) + } + + n, err := db.CountActivitiesMissingWorkout(ctx, userID) + if err != nil { + t.Fatalf("CountActivitiesMissingWorkout: %v", err) + } + if n != 1 { + t.Fatalf("CountActivitiesMissingWorkout = %d, want 1 (only activity b)", n) + } + + pending, err := db.ActivitiesMissingWorkout(ctx, userID, 10) + if err != nil { + t.Fatalf("ActivitiesMissingWorkout: %v", err) + } + if len(pending) != 1 || pending[0].ID != idB { + t.Fatalf("ActivitiesMissingWorkout = %+v, want only activity b (id %d)", pending, idB) + } + + confirmed, _, err := db.GetActivity(ctx, userID, idA) + if err != nil { + t.Fatalf("GetActivity (a): %v", err) + } + if confirmed.WorkoutNotFoundAt == nil { + t.Error("activity a's WorkoutNotFoundAt is nil, want it set") + } + if confirmed.WorkoutRawJSON != nil { + t.Error("activity a's WorkoutRawJSON should stay nil -- not-found is recorded separately, not faked") + } +} +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `cd backend && go test ./internal/store/... -run TestActivitiesMissingWorkout_ExcludesConfirmedNotFound -v` +Expected: FAIL to compile (`db.SetActivityWorkoutNotFound` and `Activity.WorkoutNotFoundAt` +undefined). + +- [ ] **Step 3: Add the schema column** + +In `backend/internal/store/schema.sql`, replace: + +```sql + -- Genuine raw get_workout_by_id() response, the source used to compute + -- alignWorkoutTargets. Null when the activity has no workout_id. + workout_raw_json TEXT, + created_at TEXT NOT NULL DEFAULT (datetime('now')), +``` + +with: + +```sql + -- Genuine raw get_workout_by_id() response, the source used to compute + -- alignWorkoutTargets. Null when the activity has no workout_id. + workout_raw_json TEXT, + -- Set when get_workout_by_id returned a definitive HTTP 404 (the + -- workout was deleted on Garmin's side after being linked to this + -- activity) -- distinct from workout_raw_json staying null for "not yet + -- fetched": this activity is excluded from ActivitiesMissingWorkout so + -- it stops being retried forever (see + -- docs/superpowers/specs/2026-07-27-workout-not-found-design.md). + -- workout_raw_json itself is never fabricated; it just stays null. + workout_not_found_at TEXT, + created_at TEXT NOT NULL DEFAULT (datetime('now')), +``` + +- [ ] **Step 4: Update the Go struct, column list, and scan** + +In `backend/internal/store/activities.go`, replace: + +```go + // WorkoutRawJSON is the genuine raw get_workout_by_id() response for this + // activity's structured workout -- the source used to compute each lap's + // TargetPaceLowMps/HighMps and TargetHRLowBpm/HighBpm (see + // internal/sync/mapping.go's alignWorkoutTargets). Nil when the activity + // has no WorkoutID, or was synced before this column existed. + WorkoutRawJSON *string + CreatedAt string + UpdatedAt string +} +``` + +with: + +```go + // WorkoutRawJSON is the genuine raw get_workout_by_id() response for this + // activity's structured workout -- the source used to compute each lap's + // TargetPaceLowMps/HighMps and TargetHRLowBpm/HighBpm (see + // internal/sync/mapping.go's alignWorkoutTargets). Nil when the activity + // has no WorkoutID, or was synced before this column existed. + WorkoutRawJSON *string + // WorkoutNotFoundAt is set when get_workout_by_id returned a definitive + // HTTP 404 for this activity's WorkoutID -- distinct from + // WorkoutRawJSON staying nil for "not yet fetched" (see + // SetActivityWorkoutNotFound). + WorkoutNotFoundAt *string + CreatedAt string + UpdatedAt string +} +``` + +Replace: + +```go +func scanActivity(row interface{ Scan(...any) error }) (Activity, error) { + var a Activity + err := row.Scan( + &a.ID, &a.GarminActivityID, &a.EventTypeKey, &a.WorkoutID, &a.StartTimeUTC, + &a.DurationSeconds, &a.DistanceMeters, &a.AvgHR, &a.MaxHR, + &a.AvgSpeedMps, &a.ElevationGainM, + &a.AerobicTrainingEffect, &a.AnaerobicTrainingEffect, &a.VO2MaxValue, + &a.RawJSON, &a.DetailsFetchedAt, &a.DetailsRawJSON, &a.SplitsFetchedAt, &a.WorkoutRawJSON, + &a.CreatedAt, &a.UpdatedAt, + ) + return a, err +} + +const activityColumns = ` + id, garmin_activity_id, event_type_key, workout_id, start_time_utc, + duration_seconds, distance_meters, avg_hr, max_hr, + avg_speed_mps, elevation_gain_m, + aerobic_training_effect, anaerobic_training_effect, vo2max_value, + raw_json, details_fetched_at, details_raw_json, splits_fetched_at, workout_raw_json, + created_at, updated_at +` +``` + +with: + +```go +func scanActivity(row interface{ Scan(...any) error }) (Activity, error) { + var a Activity + err := row.Scan( + &a.ID, &a.GarminActivityID, &a.EventTypeKey, &a.WorkoutID, &a.StartTimeUTC, + &a.DurationSeconds, &a.DistanceMeters, &a.AvgHR, &a.MaxHR, + &a.AvgSpeedMps, &a.ElevationGainM, + &a.AerobicTrainingEffect, &a.AnaerobicTrainingEffect, &a.VO2MaxValue, + &a.RawJSON, &a.DetailsFetchedAt, &a.DetailsRawJSON, &a.SplitsFetchedAt, &a.WorkoutRawJSON, + &a.WorkoutNotFoundAt, &a.CreatedAt, &a.UpdatedAt, + ) + return a, err +} + +const activityColumns = ` + id, garmin_activity_id, event_type_key, workout_id, start_time_utc, + duration_seconds, distance_meters, avg_hr, max_hr, + avg_speed_mps, elevation_gain_m, + aerobic_training_effect, anaerobic_training_effect, vo2max_value, + raw_json, details_fetched_at, details_raw_json, splits_fetched_at, workout_raw_json, + workout_not_found_at, created_at, updated_at +` +``` + +- [ ] **Step 5: Add `SetActivityWorkoutNotFound` and update the two query methods** + +In `backend/internal/store/activities.go`, right after `SetActivityWorkout`, add: + +```go + +// SetActivityWorkoutNotFound records that get_workout_by_id returned a +// definitive HTTP 404 for this activity's WorkoutID -- the workout was +// deleted on Garmin's side after being linked to this activity. +// WorkoutRawJSON is deliberately left nil (never fabricated); this is a +// separate marker so ActivitiesMissingWorkout stops retrying it forever. +func (db *DB) SetActivityWorkoutNotFound(ctx context.Context, userID, activityID int64) error { + _, err := db.ExecContext(ctx, ` + UPDATE activities SET workout_not_found_at = datetime('now'), updated_at = datetime('now') + WHERE id = ? AND user_id = ?`, activityID, userID) + if err != nil { + return fmt.Errorf("set activity %d workout not found for user %d: %w", activityID, userID, err) + } + return nil +} +``` + +Replace: + +```go +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 AND details_fetched_at IS NOT NULL + ORDER BY start_time_utc DESC LIMIT ?`, userID, limit) +``` + +with: + +```go +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 + AND details_fetched_at IS NOT NULL AND workout_not_found_at IS NULL + ORDER BY start_time_utc DESC LIMIT ?`, userID, limit) +``` + +Replace: + +```go +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 AND details_fetched_at IS NOT NULL`, userID).Scan(&n) +``` + +with: + +```go +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 + AND details_fetched_at IS NOT NULL AND workout_not_found_at IS NULL`, userID).Scan(&n) +``` + +- [ ] **Step 6: Run the test to verify it passes** + +Run: `cd backend && go test ./internal/store/... -run TestActivitiesMissingWorkout -v` +Expected: PASS, including both the existing test and the new one. + +- [ ] **Step 7: Regenerate the schema doc** + +Run: `cd backend && go run ./cmd/dumpschema` +Expected: `docs/DATABASE.md` updates to include `workout_not_found_at` in the `activities` table. + +- [ ] **Step 8: Run the full backend suite** + +Run: `cd backend && go build ./... && go vet ./... && gofmt -l . && go test ./...` +Expected: all pass, `gofmt -l .` prints nothing. + +- [ ] **Step 9: Commit** + +```bash +git add backend/internal/store/schema.sql backend/internal/store/activities.go backend/internal/store/store_test.go docs/DATABASE.md +git commit -m "$(cat <<'EOF' +feat(store): add workout_not_found_at, exclude it from missing-workout queries + +A confirmed-404 workout must stop being retried forever, but +workout_raw_json should never be fabricated -- it stays null exactly +as it does for "not yet fetched." workout_not_found_at is the +separate marker ActivitiesMissingWorkout/CountActivitiesMissingWorkout +now check for, mirroring the existing pattern of the other _fetched_at +columns. UpsertActivity's ON CONFLICT clause already never touches +these columns, so the marker persists across every later re-sync. +EOF +)" +``` + +--- + +### Task 4: `fillPendingWorkouts` stops retrying a confirmed 404 + +**Files:** +- Modify: `backend/internal/sync/service.go` (`fillPendingWorkouts`) +- Modify: `backend/internal/sync/service_test.go` (new test) + +**Interfaces:** +- Consumes: `garmin.ErrNotFound` (Task 2), `db.SetActivityWorkoutNotFound` (Task 3). +- Produces: nothing new -- this is the final integration point for this fix. + +- [ ] **Step 1: Write the failing test** + +Add to `backend/internal/sync/service_test.go`, right after +`TestFillPendingDetails_OneWorkoutFetchFailureDoesNotAbortTheWorkoutsPass`: + +```go +func TestFillPendingDetails_NotFoundWorkoutStopsBeingRetried(t *testing.T) { + db := openTestDB(t) + ctx := context.Background() + userID := provisionTestUser(t, db) + + const missingWorkoutID = 300 + missingPtr := int64(missingWorkoutID) + m := &mock.Client{ + Activities: []garmin.Activity{ + {ActivityID: 1, ActivityType: garmin.ActivityType{TypeKey: "running"}, WorkoutID: &missingPtr, + StartTimeGMT: "2026-07-01 06:00:00", Distance: 5000, Duration: 1500}, + }, + Splits: map[int64]garmin.ActivitySplits{ + 1: {ActivityID: 1, Laps: []garmin.Lap{{LapIndex: 1, IntensityType: "ACTIVE"}}}, + }, + Details: map[int64]garmin.ActivityDetails{1: {ActivityID: 1}}, + Workouts: map[int64]garmin.Workout{}, + WorkoutErrByID: map[int64]error{missingWorkoutID: fmt.Errorf("API Error 404: %w", garmin.ErrNotFound)}, + } + 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 confirmed-404 workout: %v", err) + } + + remaining, err := db.CountActivitiesMissingWorkout(ctx, userID) + if err != nil { + t.Fatalf("CountActivitiesMissingWorkout: %v", err) + } + if remaining != 0 { + t.Fatalf("CountActivitiesMissingWorkout = %d, want 0 (confirmed-404 activity must not be retried)", remaining) + } + + activities, err := db.ListActivities(ctx, userID, store.ActivityFilter{}) + if err != nil { + t.Fatalf("ListActivities: %v", err) + } + if len(activities) != 1 { + t.Fatalf("ListActivities returned %d activities, want 1", len(activities)) + } + got, _, err := db.GetActivity(ctx, userID, activities[0].ID) + if err != nil { + t.Fatalf("GetActivity: %v", err) + } + if got.WorkoutNotFoundAt == nil { + t.Error("WorkoutNotFoundAt is nil, want it set") + } + if got.WorkoutRawJSON != nil { + t.Error("WorkoutRawJSON should stay nil -- not-found is recorded separately, not faked") + } + + // A second FillPendingDetails call must not attempt this workout again + // (it's no longer in ActivitiesMissingWorkout's result set at all). + if err := svc.FillPendingDetails(ctx, 10); err != nil { + t.Fatalf("second FillPendingDetails: %v", err) + } +} +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `cd backend && go test ./internal/sync/... -run TestFillPendingDetails_NotFoundWorkoutStopsBeingRetried -v` +Expected: FAIL -- `CountActivitiesMissingWorkout` still returns 1 (today's code retries forever +regardless of the error's nature). + +- [ ] **Step 3: Update `fillPendingWorkouts`** + +In `backend/internal/sync/service.go`, replace: + +```go + 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 +} +``` + +with: + +```go + 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 { + if errors.Is(err, garmin.ErrNotFound) { + // A definitive 404 (the workout was deleted on Garmin's side + // after being linked to this activity) will never succeed on + // retry -- mark it so ActivitiesMissingWorkout stops + // surfacing it, instead of retrying forever. + log.Printf("sync: workout for activity %d not found on Garmin, marking as such (will not retry): %v", a.GarminActivityID, err) + if serr := s.db.SetActivityWorkoutNotFound(ctx, s.userID, a.ID); serr != nil { + return fmt.Errorf("mark activity %d workout not found: %w", a.GarminActivityID, serr) + } + } else { + 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 +} +``` + +Add `"errors"` to `service.go`'s import block. + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `cd backend && go test ./internal/sync/... -run TestFillPendingDetails_NotFoundWorkoutStopsBeingRetried -v` +Expected: PASS. + +- [ ] **Step 5: Run the full `internal/sync` test suite** + +Run: `cd backend && go test ./internal/sync/... -v 2>&1 | tail -60` +Expected: every test passes, including +`TestFillPendingDetails_OneWorkoutFetchFailureDoesNotAbortTheWorkoutsPass` unchanged (its failure +is a plain `fmt.Errorf("garmin says no")`, not `ErrNotFound`-wrapped, so it still hits the +retry-next-sync branch exactly as before). + +- [ ] **Step 6: Run the full backend suite** + +Run: `cd backend && go build ./... && go vet ./... && gofmt -l . && go test ./...` +Expected: all pass, `gofmt -l .` prints nothing. + +- [ ] **Step 7: Commit** + +```bash +git add backend/internal/sync/service.go backend/internal/sync/service_test.go +git commit -m "$(cat <<'EOF' +fix(sync): stop retrying a workout Garmin confirms is gone + +fillPendingWorkouts previously treated every get_workout_by_id +failure identically -- log it, leave workout_raw_json null, retry +next sync -- which is correct for a transient failure but means a +definitive 404 (the workout deleted on Garmin's side after being +linked to an activity) got silently retried forever, with the only +symptom being an unexplained, permanent "1 more workout pending" +nudge. Now checks errors.Is(err, garmin.ErrNotFound) and, only for +that case, calls SetActivityWorkoutNotFound instead of retrying. +EOF +)" +``` + +--- + +## Final verification + +- [ ] Run the full backend suite one more time: `cd backend && go build ./... && go vet ./... && gofmt -l . && go test ./...` +- [ ] Run the Python wrapper tests one more time: `cd backend/internal/garmin/pyscript && python3 -m pytest tests/ -v` +- [ ] Confirm `docs/DATABASE.md` reflects the new `workout_not_found_at` column. +- [ ] Use superpowers:finishing-a-development-branch to wrap up (tests green -> present the + merge/PR/keep-as-is menu).