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 <noreply@anthropic.com>
This commit is contained in:
844
docs/superpowers/plans/2026-07-27-workout-not-found-plan.md
Normal file
844
docs/superpowers/plans/2026-07-27-workout-not-found-plan.md
Normal file
@@ -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).
|
||||
Reference in New Issue
Block a user