Skip to content

Code review 2026-06-30: tracking + remediation checklist #75

Description

@kellylford

Tracking issue for the comprehensive iOS code review (2026-06-30). Detail in iOS/CODE_REVIEW_2026-06-30.md; VoiceOver items in iOS/VOICEOVER_VERIFICATION.md.

All work is on branch code-review-fixes so main stays shippable. Build + 101 tests pass at every step. Tested on-device once (all good); a second test pass is worthwhile for the latest batch.

✅ Done (on code-review-fixes)

Correctness / crash-safety: CR-1 (settingsVersion encode/decode) · CR-2 (App Group suite) · CR-3 (bounds-safe array access) · HI-8 (stale-hour fallback)
Cost / caching: HI-1 (WeatherKit condition cached by coordinate; overlay preserved) · HI-6 (offset-aware cache validity) · HI-5 (My Data add invalidates cache instead of API storm) · H2/L4 (marine cache trim + cleanup)
Sync / persistence: HI-4 (iCloud cities last-writer-wins by timestamp + quota logging) · M3 (HistoricalWeatherCache → Caches dir) · L7 (iCloud decode logging) · M7 (timezone fallback to UTC)
Robustness: HI-3 partial (alert failures logged; UI surfacing still deferred) · LocationService permission race · M1 (regional cache write off the lock) · M5 (deduped detailCategories defaults)
Docs / a11y: VO-4 (sort selected state) · M4 (CLAUDE.md flag table)

Tests added for CR-1 and CR-3.

❌ Withdrawn after trust-but-verify

CR-5 (radar timeline — intentional) · HI-2 (dew point — reviewer backwards, decodes correctly).

🔇 VoiceOver — complete (user-tested); VO-7 text sizing → #76

⏸️ Deferred — remaining

  • HI-7 Re-key the weather cache on stable coordinates instead of per-install UUID. High churn (touches every cache call site) for a marginal cost benefit; deferred deliberately.
  • HI-10 Second WeatherService() in WeatherAroundMeView — explicit "intentional cache isolation" comment; revisit only if cost data warrants.
  • HI-3 (UI part) Surface a distinct "couldn't check alerts" state — needs a UX/VoiceOver decision (all consumers currently catch-and-ignore).
  • TableView rebuilds the a11y tree every scroll frame — touches the VoiceOver data-table bridge, so deferred pending your OK (VoiceOver-adjacent).
  • Refactor: shared WeatherFormatter + decompose 2,707-line CityDetailView — large; its own focused effort. Resolves cross-view precision drift.
  • WeatherCache.swift is dead code (only a commented reference) — safe to delete but needs pbxproj surgery; left in place.

🧹 Cruft (needs your OK)

  • DeveloperSettingsView.swift.bak, TestApp/, TestWeatherFastApp/ tracked but not in the build. TestApp/ has a TUTORIAL.md you may have authored — confirm before deleting.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions