workouts/REVIEW-2026-07-07.md

# Workouts — Full Review Findings (2026-07-07)

Working document from the four-agent review (UX, correctness, indie-sdlc conformance,
indie-skills currency). Check items off as they're resolved; decisions recorded inline.

**Decisions so far**
- [x] **H1 (watch↔phone clobber): noted, deliberately deferred** — do not fix yet.
- [x] **H2: decided WATCH-ONLY and implemented** — phone estimate path removed (see H2 below).
- [x] **Batch 2 (2026-07-07 evening, all uncommitted):** H2 watch-only, watchOS test target
  (19 tests), UX #5/#8/#9, site status → released, guidebook IndieSync pin → 0.3.0,
  IndieBackup + IndieDiag integrated, build-stamping unified across all four targets.
- [x] **Test-gap closure: done** — 33 new tests (81 total green), fixes for
  M1/M2/M3, L2/L3, Bird Dog seed, dev-tool gating.
- [x] **2026-07-08: everything committed & pushed** — 9 commits on `main`
  (`a93cbc3`…`495fce1`): the other session's library-add feature, the fix+test batch,
  H2 watch-only, watchOS tests, UX #5/#9, Live Activity, IndieBackup, IndieDiag, and
  the changelog/README docs commit. Site (`released`) and guidebook (IndieSync 0.3.0)
  fixes pushed in their own repos. "Uncommitted" annotations below are now historical.
  Still untracked on purpose: this file, and the other session's new morning-mobility
  `Exercise Library/` folders (in progress, not ours to commit).

---

## 1. Correctness & robustness

### HIGH

- [x] *(RESOLVED-FOR-NOW 2026-07-08, uncommitted, in two steps. Step 1 — trigger (a)
  closed by the L1 write-queue work: `ingestFromWatch` gates on `updatedAt` against the
  cache (which mirrors every accepted write immediately, so it upper-bounds any pending
  slot): strictly-older deliveries dropped with an authoritative re-push,
  equal-timestamp echoes ignored, queued-but-unwritten deletes veto like a tombstone.
  Step 2 — trigger (b) narrowed by the "cheap fix": the watch's `WorkoutLogListView`
  now absorbs phone edits while open (`onChange(of: workout.updatedAt)` re-seed,
  strictly-newer guard so an echo or a stale in-flight push never regresses the screen;
  clock skew degrades to a skipped absorb, never a clobber). Hard "primacy baton"
  considered and REJECTED — handoff over the unordered WC channel is its own consensus
  problem, and offline watch editing must stay possible, which reintroduces merge anyway.
  **Durable fix documented, deliberately not built** (revisit if users report vanished
  sets after reconnect): per-log merge — optional per-log `updatedAt` on
  `WorkoutLogDocument` (additive, like startedAt/completedAt), merge incoming docs
  log-by-log instead of replacing, explicit add/remove intents so an absent log is never
  ambiguous. Makes edits to different logs commute; closes the concurrent-offline-edit
  loss that the absorb can't (whole-doc exchange after a disconnection still loses one
  side's batch). Full rationale in the `ingestFromWatch` comment block.)*
  **H1 — Watch↔phone workout edits clobber each other** *(was DEFERRED 2026-07-07)*
  `SyncEngine.swift:215-225, 332-346`; `Workouts Watch App/Connectivity/WatchConnectivityBridge.swift:184-199`
  Saves are last-writer-by-arrival; no `updatedAt` guard anywhere on the apply path.
  Two triggers: (a) the watch mixes `sendMessage` with a `transferUserInfo` fallback,
  which are unordered relative to each other — a failed-then-queued older edit can land
  after a newer one and win; (b) the exclusive-edit lock covers only the phone detail
  editor, not the phone list checkboxes or run flow, and the watch's `WorkoutLogListView`
  seeds its doc once at init with no `onChange(of: workout.updatedAt)` absorb (the phone
  list has one at line 168) — simultaneous two-device use deterministically loses one
  side. **Fix when picked up:** apply-side `updatedAt` guard (or per-log merge), watch
  absorb, single ordered channel for durable updates.

- [x] *(RESOLVED: watch-only, implemented uncommitted — the phone-side MET-estimate write
  path is gone; `WorkoutHealthWriter` → `WorkoutHealthDeleter`, which only best-effort
  deletes legacy phone-authored estimates, now with an auth guard + os.Logger (also closes
  L4). Phone share auth slimmed to `workoutType()`; phone read auth removed entirely.
  Legacy `phoneEstimate` docs still decode and render with their "estimated" caption.)*
  **H2 — HealthKit double-write / orphaned estimate**
  `WorkoutHealthWriter.swift:31, 63-77, 122-169`; `Workouts Watch App/Views/ActiveWorkoutGateView.swift:106-128`
  Dedupe between the phone's MET-estimate and the watch's real `HKWorkout` is only a
  10 s debounce. Watch unreachable/slow at finish → both write (duplicate Move-ring
  credit); when watch metrics later replace the stored UUID, the phone's estimate is
  orphaned in Health and unreachable from the app's delete path. `sweepRecentlyCompleted`
  can re-arm the race at next launch.
  **Options:** (a) watch-only Health writes — simplest, but watchless users lose Health
  integration entirely; (b) ownership-at-start — if the phone launched a watch session
  for this workout, the watch owns the Health write and the phone never estimates it;
  estimate only for workouts that never had a watch session. (b) keeps phone-only value
  and kills the race at the root. Either way: delete a superseded estimate if one exists.

### MEDIUM (fixes in flight via the test-gap agent)

- [x] *(fixed, uncommitted)* **M1 — Timezone re-bucketing orphans the old workout file**
  `Shared/Model/Documents.swift:97-103` + `SyncEngine.swift:335`
  `relativePath` derives `Workouts/YYYY/MM/` from `start` using the device's *current*
  timezone. A tz change that flips `start`'s local month means a later save writes a new
  bucket file and never removes the old one — two files, one ULID (the cache shows one
  row; "Clean Up Duplicates" exists to mop these up). Fix: on save, if the computed path
  differs from the cached `jsonRelativePath`, remove the old file.

- [x] *(fixed, uncommitted — stub re-check added; name guard deliberately NOT added: on a
  legitimate upgrade the seed itself is the live split by that name)* **M2 — `reconcileSeeds`
  `.upgrade` skips the tombstone re-check `.write` does**
  `SyncEngine.swift:592-639`
  `.write` re-checks `stubExists(id:)` + name guard right before writing (628-630);
  `.upgrade` (621-624) doesn't. A user forking a seed during the 5 s settle window can
  get the just-deleted seed transiently resurrected as a duplicate. Self-heals, but
  visible. Fix: symmetric re-check before `writeSeedBytes`.

- [x] *(fixed, uncommitted)* **M3 — Watch never prunes on a legitimately-empty phone push**
  `Workouts Watch App/Connectivity/WatchConnectivityBridge.swift:213-239`
  `applyState` bails on `splits.isEmpty && workouts.isEmpty` *before* the prune loop,
  but the decode-failure case is already handled by the separate nil guard — so this
  guard only suppresses the genuine "user deleted everything" state, leaving phantom
  splits on the wrist forever. Fix: drop the isEmpty guard.

### LOW (fixes in flight)

- [x] *(BUILT 2026-07-08, uncommitted — `WriteBacklog.swift` (pure slot queue + backoff
  policy + sidecar persistence), `SyncEngine` mirror-first save/delete + serial drain
  loop, `SyncStatusBanner` on the root screen, backlog cleared in `beginRestore()` and
  on account wipe (`WorkoutsModelContainer`), flush-on-background in `WorkoutsApp`.
  Also implements H1's phone-side `updatedAt` intake gate in `ingestFromWatch` — stale
  watch deliveries dropped + authoritative re-push, equal-timestamp echoes ignored,
  pending-delete veto honored. Import/reconcile/observer-removal all skip documents
  superseded by a pending slot, and reconcile's prune exempts pending ids. H1's
  watch-side absorb remains deferred.)*
  **L1 — Sync errors visible only in Settings.** `SyncEngine.swift` `report()`,
  `SettingsView.swift:128`. A failed save on the main screen is silent: the view's local
  doc state shows the change, the write fails before the cache upsert, and the edit
  evaporates on navigation with zero feedback. `lastSyncError` renders only in Settings.
  **Agreed design (2026-07-08) — write-through slot queue + tiered banner:**
  route ALL writes (view saves, watch ingest, deletes-as-tombstone-markers) through a
  per-document-id single slot (map id → latest pending doc; per-ID, NOT per-type — two
  different offline-edited docs must not evict each other); the drain loop serializes
  writes per id, so in-flight races are structurally impossible and a failed write just
  stays in its slot; slot replacement compares `updatedAt` and keeps the newer doc —
  which is a contained version of H1's phone-side apply guard for free (watch-side
  absorb still separate/deferred); clear slots in `beginRestore()` + account wipe;
  persist slots (Application Support, not the iCloud container) with a ~24 h TTL.
  Banner tiers: 0 = silent immediate retries (~10 s); 1 = calm "Changes pending —
  retrying", clears on success; 2 = prominent fault → Diagnostics, on backoff
  exhaustion OR immediately for known-unrecoverable errors (disk full, container gone).
- [x] *(fixed, uncommitted)* **L2 — `report()` uses `print` not the engine's `os.Logger`.**
- [x] *(fixed, uncommitted — `end` now stamped only on the transition into completed)*
  **L3 — `recomputeStatusFromLogs` re-stamps `end = Date()` on every resolve.**
- [x] *(fixed with H2 — auth guard + os.Logger in `WorkoutHealthDeleter`)*
  **L4 — `deleteFromHealth` had no auth check and swallowed all errors.**
- [x] *(fixed, uncommitted — second Bird Dog → Crunch 3×12, regenerated via the script;
  split ULID unchanged, other seeds byte-identical)* **Seed data bug — "Bodyweight Core"
  lists Bird Dog twice.**
- [x] *(fixed, uncommitted — wrapped in `#if DEBUG`)* **"Developer → Clean Up Duplicates"
  ships un-gated to all users.**

### Test gaps (closure in progress)

- [~] `SyncEngine` orchestration — only pure logic covered (`currentSplitID` redirect +
  the planners); real save/reconcile paths need an injection seam for `DocumentFileStore`
  (resolved inside `connect()` today) — deliberate scope stop, noted in `SyncEngineTests.swift`.
- [x] Workout mapper round-trip — `WorkoutDocumentMapperTests.swift` (4 tests: full field
  fidelity incl. machineSettings/metrics/hrZoneSeconds, log order, by-id reconciliation).
- [x] Status machine — `WorkoutStatusMachineTests.swift` (11 tests incl. L3 regression).
- [x] `WCPayload` — `WCPayloadRoundTripTests.swift` (11 tests, every wire shape incl.
  liveProgress and the nil-vs-empty-vs-corrupt decode contract).
- [x] Month bucketing — `WorkoutPathBucketingTests.swift` (5 tests incl. the M1 condition).
- [x] *(done, uncommitted)* watchOS test target "Workouts Watch AppTests" — 19 tests in 3
  suites: `WatchCacheApplier` (upsert/prune seam extracted from the bridge, incl. the
  empty-push prune fix and nil-decode-must-not-prune), bridge optimistic updates, HR-zone
  math + watch helpers. H1 territory deliberately untested pending the deferral.

**Verified sound (checked, not bugs):** no double-seed from slow metadata at launch
(seeding reads `DocumentFileStore.list()` directly + settle re-check); eviction-safe
reads (unreadable ≠ deleted); tombstone veto on both import paths; observer re-apply
idempotent; HKWorkoutSession double-finish/system-end guarded; account-change and
schema-bump cache wipes handled.

---

## 2. User experience

### Top 10 by impact

- [ ] **1. The "log" is the plan — actuals aren't recordable.** `WorkoutLogDocument`
  holds one planned sets/reps/weight + `currentStateIndex`. Did 10/8/6 with a weight
  bump? Uncapturable. The progression chart plots *planned* weight of completed logs
  (`WeightProgressionChartView.swift:36`), so "progress" only moves when the plan is
  edited. Add per-set actual entries (`[SetEntry]` reps/weight/RPE) captured on the work
  page; drive chart + volume from actuals. *The identity-defining fix for a tracker.*
- [ ] **2. No custom exercises** — every add path funnels through `ExercisePickerView`
  listing only the 47 bundled `ExerciseMotionLibrary.exerciseNames`
  (`ExercisePickerView.swift:26`; `ExerciseAddEditView.swift:60-75`). Allow free-text
  names; degrade gracefully to no figure (the figure slot already handles absence).
- [ ] **3. Integer-only weight; unit switch relabels without converting.** Weight is
  `Int` (`Documents.swift:49,151`), wheels step by 1 (no 2.5/1.25 for kg/micro-loading),
  and toggling Weight Unit turns `45 lb` into `45 kg` silently (`Enums.swift:96-116`).
  Store `Double`, offer fractional steps, convert on unit change.
- [ ] **4. Exercise picker is a flat 47-row list** — no search, no grouping, no figure
  thumbnails or target chips, despite the app owning rich per-exercise content. Add
  `.searchable` + group by target/equipment.
- [x] *(fixed, uncommitted — shared `StartedWorkoutNavigator` view modifier now drives
  both start paths; sheet dismisses, parent polls the cache and pushes into the workout)*
  **5. "Start New" dumps you back on the log list.**
- [ ] **6. Splits live three taps deep in Settings** (gear → Library → Splits). Routine
  management needs a primary home on the main screen.
- [ ] **7. Watch can't start a workout; offline Refresh is a silent no-op.**
  `ActiveWorkoutGateView.swift:142-154`; `requestSync()` is reachability-gated and
  returns silently (`WatchConnectivityBridge.swift:109-114`). Allow starting a cached
  split from the wrist (queue via `transferUserInfo`); give Refresh a "Phone
  unreachable" state.
- [x] *(done, uncommitted — new "Workouts Widget" extension target: lock-screen +
  Dynamic Island compact/minimal/expanded, driven by the same run-flow transitions as the
  watch's `LiveProgress` mirror; wall-clock `Text(timerInterval:)` countdowns so no push
  updates; stale activities cleaned at launch; degrades silently when disabled. Bonus:
  build stamping unified via a `scriptAnchors` YAML anchor across all four targets, fixing
  the watch-widget CFBundleVersion warning.)* **8. No Live Activity / Dynamic Island.**
- [x] *(fixed, uncommitted — checkbox gets label/value/hint; HR-zone bar is one spoken
  element + visible numbered legend (no longer color-only); run-flow display fonts are
  `@ScaledMetric` (Dynamic Type); figure `accessibilityHidden(true)`; CalendarListItem
  reads as one full date; Settings gear labeled)* **9. Accessibility gaps on custom
  controls.**
- [ ] **10. No real progress/history** — one per-exercise planned-weight chart; no PRs,
  streaks, volume trend, calendar, workout counts. The main between-session reason to
  open a fitness app.

### Other UX findings by area

**First-run:** iCloud gate copy is best-in-class — keep. Nothing tells a new user that
4 starter splits exist or where they live; add a one-time hint.

**Workout flow:**
- [ ] Active workout looks like any other row — no pinned "Resume" banner (`WorkoutLogsView.swift:33-38`).
- [ ] `PlanEditView` load-type control omits "None" (bodyweight logs can't stay bodyweight; `PlanEditView.swift:65-70`); its ranges disagree with `ExerciseAddEditView` (sets 1...7 vs 0..<20, reps 1...40 vs 0..<100). Unify.
- [ ] `ExerciseAddEditView` allows a meaningless 0-set exercise (`ForEach(0..<20)`, line 82).
- [ ] Rest is one global setting; no per-exercise rest.
- [ ] No haptic on the list checkbox (feedback only exists inside the run flow).
- [ ] Notes only reachable from the exercise detail screen, not the run flow where you'd jot "felt heavy."

**Split/exercise management:**
- [ ] Two near-duplicate exercise-editing surfaces (`SplitDetailView` inline list vs `SplitAddEditView` → `ExerciseListView`) with different subtitle formats. Collapse to one.
- [ ] "What is a Split?" explainer renders on every visit (`SplitDetailView.swift:61-64`) — make it empty-state-only or dismissible.
- [ ] Reorder exists but has no visible drag affordance.
- [ ] Adding one exercise via multi-select picker silently takes 3×10×40 defaults; no plan-editor handoff (abandoned intent noted at `SplitDetailView.swift:190-192`).

**Watch:** live mirroring/Always-On/HR zones are strong. Only a launcher complication
exists — an accessory-rectangular live complication (set N of M, rest countdown) would
be high value.

**Settings:**
- [ ] Workout-section footer only describes auto-finish and says "the watch" though it now applies to phone too (`SettingsView.swift:53-55`).
- [ ] No unit-conversion warning; no backup/export entry (see IndieBackup below).
- "Restore Starter Splits" inline confirmation count is a good pattern — extend to other bulk actions.

**Missing table-stakes features (backlog candidates):** supersets/circuits, warm-up
sets, plate math, rest-day/streak tracking, Siri/App Intents ("start Leg Day"), iPhone
home/lock-screen widgets, bodyweight tracking. (Duration + volume for finished workouts
are already done well.)

**Accessibility (additional):** `CalendarListItem` reads as three disjoint VoiceOver
elements (`CalendarListItem.swift:23-31`).

---

## 3. Release & SDLC conformance

Verified clean: XcodeGen setup, Swift 6 strict concurrency zero-warning build, 48/48
tests green (fresh DerivedData, pinned sim), build stamping live, IndieSync 0.3.0 /
IndieAbout 0.2.2 pinned by semver and matching heads, entitlements exactly per the 2.0
architecture, changelog/README/LICENSE compliant, release pipeline per spec, web page +
stoop present, git hygiene clean.

- [ ] **Ship state: 2.0 is live; ~11 months unshipped.** ASC shows one version, 2.0
  `READY_FOR_SALE`, last build 2025-08-09. Local is 2.2 with three unshipped changelog
  months (exercise library, machine settings, schema v2/v3). Decide when to cut 2.2.
- [x] *(fixed, uncommitted in the Websites repo — `status: released`, matching sibling
  apps and the zod schema)* **Site status field wrong.**
- [ ] **CLAUDE.md Build section stale** (repeat finding, already logged in APPS.md): no
  pinned `xcodebuild build`/`test -destination 'id=815B7FED-…'` lines, no
  `## Key Dependencies` section, stray HTML comment under the title.
- [x] *(done, uncommitted — IndieBackup 2.0.0; `backupRoot` = the container's `Documents/`
  incl. `Stubs/` tombstones so restores can't resurrect deletes; restore tears down the
  MetadataObserver behind a new `SyncEngine.isRestoring` guard (connect/reconcile bail
  during restore), then rebuilds the cache via reconcile and re-baselines a fresh
  observer; `BackupsSectionView` in Settings; `.workoutsbackup` doc type + `onOpenURL`
  tap-to-restore)* **IndieBackup not integrated.**
- [ ] **SyncEngine pre-connect writes silently no-op** instead of throwing
  `SyncError.iCloudUnavailable` (architecture.md invariant 5). Outcome holds via the
  RootGateView hard gate; flag only if SyncEngine is touched again.
- [x] *(fixed, uncommitted in the indie-sdlc repo — APPS.md pin corrected to 0.3.0 with
  the `prune(exempting:)` rationale; componentry.md table gained the 0.3.0 entry, verified
  against the indie-sync 0.3.0 tag message)* **Guidebook-side drift.**

---

## 4. indie-skills currency

Up to date (verified): icloud-sync-engine (IndieSync 0.3.0 = latest tag),
indie-about (0.2.2 = latest), app-changelog (one-liner rule compliant),
swiftui-app-skeleton (Workouts is the skill's canonical example), indie-website
(decommissioned-into-indie.rzen.dev model), stoop-space (config + frontmatter live),
skill deploy mechanism, app-versioning (functionally current).

- [ ] **appstore-publish — behind (skill changed 2026-07-07):** add
  `Scripts/metadata/app/pricing.json` (`{"baseTerritory":"USA","price":"0.00"}`) and
  `availability.json` (`{"availableInNewTerritories":true,"territories":"ALL"}`) or the
  next metadata push won't manage price/territories.
- [x] *(done, uncommitted — five reference files copied to `Workouts/Diagnostics/`;
  `SchemaSkipScanner` adapted with a per-file ceiling resolver because Workouts has two
  independent document schema versions (Splits=2, Workouts=3; Stubs never counted);
  Diagnostics row inside the Settings iCloud Sync section: account/container, download/
  eviction state, schema-skipped count with "update the app" hint, network readiness,
  Copy Diagnostics)* **indie-diag — not integrated.**
- [x] *(done — see §3)* **indie-backup — not integrated.**
- [x] *(done with the Live Activity work — `scriptAnchors` anchor, all four targets stamp)*
  **app-versioning nits.**

---

## Suggested order

1. Bug fixes + tests (in flight): mediums/lows above, Bird Dog seed, dev-tool gating,
   test gaps — excluding H1 (deferred) and H2 (pending decision).
2. Cheap catch-ups: pricing/availability JSONs, site status field, CLAUDE.md Build
   section, weight-unit conversion.
3. Ship 2.2.
4. Feature work by impact: per-set actuals → custom exercises + picker search → splits
   on the main screen + Start New navigation → Live Activity → IndieBackup + IndieDiag →
   progress/PR view → watch-initiated workouts → accessibility pass.