amadan

oficina / calendar Public

← Discussions

Native: a failed load keeps the events on screen (B11) open

On branch fix/r1-native-keep-events-on-error

claude operated by paul@keymail.dev session 21a32889 · claude-code/2.1.280 · MacBookPro.localdomain

Stacked on merge/native-testflight (discussion 13) — this code only exists on the native branch. Merge 13 first; this branch's diff over it is one commit.

Bug (B11, offline-audit/calendar.md)

CalendarModel.load did catch { events = []; error = … } (CalendarModel.swift:69). Any failed request (offline, 500, flaky Wi-Fi) blanked a calendar that was on screen a second earlier. The Mac menu-bar panel shares the model and reloads on every open, so opening it offline blanked the window too.

Fix

  • The catch keeps the last good calendars and events and only sets error.
  • New hasLoaded (set on success, cleared by clear() on sign-out). Never-loaded + error → the existing full-screen "Couldn't load your calendar". Loaded + error → a one-line banner, "Couldn't refresh. Showing what was last loaded." with the error text and Try again, ABOVE the day's events (window), and the same line above the agenda in the menu panel. Event count and "No events on this day" stay visible when stale.

Tests

  • New LoadFailureTests (python fixture server, same pattern as SessionLifecycleTests): fail → recover → fail. Asserts events/calendars survive the second failure, error is reported, hasLoaded semantics, and clear() still wipes.
  • Verified: restoring events = [] in the catch makes it FAIL at two assertions; passes with the fix. swift test: 6/6.
  • native/scripts/build-mac.sh and native/scripts/build-iphone.sh (xcodegen + simulator build) both succeed.
  • Not visually verified in a running app (no screenshot of the banner).
LLM

Stacked on merge/native-testflight (discussion 13). A refresh failure no longer blanks the calendar; it shows a stale banner with Try again.

claude operated by paul@keymail.dev session 21a32889 · claude-code/2.1.280 · MacBookPro.localdomain

Whole-branch review — fix/r1-native-keep-events-on-error @ b99d284 vs origin/merge/native-testflight 9ebcd95 (stacked on #13)

Reviewed by Claude Fable 5.1 (claude-fable-5-1), session 21a32889, as the AGENTS.md whole-branch reviewer. Diffed against its parent as told; scratch worktree; branch not edited.

VERDICT: APPROVE-WITH-NITS

What I checked and how

  • Diff is one commit, four Swift files, nothing outside native/apple. CalendarModel.swift, CalendarWorkspace.swift, CalendarMenuView.swift, new LoadFailureTests.swift. No Go, no migrations.
  • B11 fixed at the root. The catch no longer assigns events = []; it only sets error (still guarded by generation == token, so a stale load cannot write over a newer one). hasLoaded is set only after BOTH requests succeed, cleared by clear() on sign-out, and is private(set).
  • Views. Window: never-loaded + error → the existing ContentUnavailableView; loaded + error → a one-line banner (staleBanner) with the error text and Try again, ABOVE the day section, which is now shown whenever error.isEmpty || hasLoaded. Menu panel: same rule for the count, the "No events on this day" line and the agenda. Copy uses the same curly apostrophe as the existing string. Colour is not the only signal (icon + words).
  • Test exercises the fix — verified. swift test: 6/6 (LoadFailureTests, CivilDateTests, SessionLifecycleTests). I put events = [] back into the catch and swift test --filter LoadFailureTests fails with 2 assertions ("a failed refresh blanked the events on screen" and the dayEvents check); restored, it passes. The fixture (python ThreadingHTTPServer, /fail / /recover) follows the SessionLifecycleTests pattern.
  • Not visually verified in a running app — same as the discussion says. Given the staleBanner accessibility identifier is in place, a UI test could pick it up later.

Findings (none blocking)

  1. Nit — CalendarModel.swift:60-70. load fetches the session first and assigns calendars/weekStart/defaultCalendar before the events request. If the session call succeeds and the events call fails, the model shows FRESH calendars beside STALE events under a banner saying "showing what was last loaded". Harmless for one person on one team, but if you want the banner to be literally true, stage both responses into locals and commit them together after the second guard.
  2. Nit — CalendarModel.swift:107 (save). After a successful save, a failed reload leaves the banner up with the just-saved event absent from the list. That is honest, but notice is set from saved.warning (usually empty), so nothing says "saved". Consider notice = saved.warning ?? "Saved." or leaving quickSavedNotice-style feedback when error is non-empty.
  3. Merge order. After #13 is squash-merged, merge origin/main into this branch and confirm git diff --stat origin/main HEAD shows only these four files before its own -squash -expect (R1-BRIEF, stacked branches).