Skip to content

fix: scheduling boundaries, accurate stats, and review fixes - #7

Merged
bendns merged 3 commits into
mainfrom
fix/code-review-findings
Oct 20, 2026
Merged

bendns merged 3 commits into
mainfrom
fix/code-review-findings

Conversation

@bendns

@bendns bendns commented Oct 20, 2026

Copy link
Copy Markdown
Owner

Fixes from a full code review of the app.

Root causes and fixes

Scheduling

  • The scheduler only re-evaluated on launch, settings changes, user actions and wake. It never noticed work starting, lunch, end of day or a pause expiring, so reminders fired during lunch or after hours and none were scheduled after a late launch. ScheduleEvaluator.nextStatusChange now computes the next real status change and a timer re-evaluates at that moment. A reminder that would land after the current working stretch is not scheduled. The unused minute-by-minute nextWorkingMoment is removed.
  • Waking the Mac restarted the countdown. refresh() keeps a countdown that is still valid.
  • Snooze could schedule a reminder while paused or off-hours. It is now ignored there, and the popover disables Snooze and Skip.
  • autoStart was logged at midnight and on days off. It is now logged at the first working moment of the day.
  • Only pending notifications were cleared, so tapping a stale delivered banner could flip the posture to an outdated target. Delivered notifications are now removed too, and a fixed request identifier is used.

Stats

  • Totals stretched the last event until "now", or until midnight for past days, which inflated sitting time by hours. Totals now live in PostureStats, clipped to work hours minus lunch. Segments never cross midnight and use their start date as the id instead of a fresh UUID.

Settings / UI

  • Vacations kept the time of day and could not cover a single day. They now cover whole days.
  • After a pause expired, "Resume" stayed visible. The pause state now comes from the scheduler status.
  • The launch-at-login toggle could disagree with the system. It now reads SMAppService directly, and Settings.launchAtLogin is removed (old saved data still decodes).
  • The stand warning threshold of 90 could never trigger with a range capped at 90. It is now 45, matching the Science tab.
  • Custom mode displayed "Follow the science" as selected. Editing the shared hours wrote settings 7 times; it now writes once. A hint appears for invalid work or lunch hours. The Advanced ratio text is fixed.
  • The popover uses TimelineView for the per-second parts instead of a stored Timer.publish.

Build / CI

  • Info.plist hardcoded 1.0, so the app never showed MARKETING_VERSION and releases were mislabeled. It now uses build-setting variables.
  • make install looked for the built app before building. It now uses -derivedDataPath build.
  • Dropped the redundant CI build step. The release workflow now reads the workflow_dispatch tag from an env var instead of interpolating it into the script.

Tests: 33 to 50. New coverage for boundaries, wake, lunch clamping, snooze off-hours, autoStart, whole-day vacations and stats clipping. Removed the sleep-based wait in HistoryStoreTests and a leaked UserDefaults suite.

Not changed

Files changed

  • Hop/Scheduler/PostureScheduler.swift, ScheduleEvaluator.swift, NotificationManager.swift
  • Hop/Models/PostureStats.swift (new), Settings.swift, Presets.swift
  • Hop/Persistence/HistoryStore.swift, Hop/AppDelegate.swift, Hop/HopApp.swift
  • Hop/Menu/MenuPopoverView.swift
  • Hop/Views/History/HistoryView.swift, TodayChart.swift, WeekChart.swift
  • Hop/Views/Settings/GeneralTab.swift, ScheduleTab.swift, VacationsTab.swift, ScienceTab.swift
  • HopTests/* (new PostureStatsTests.swift)
  • project.yml, Hop/Info.plist, Makefile, .github/workflows/ci.yml, .github/workflows/release.yml

Parts of this fix were generated with Claude Code assistance.

… install

- Info.plist hardcoded 1.0, so releases ignored the MARKETING_VERSION override
- Makefile resolved the built app at parse time, before build ran
- CI: drop redundant build step; release: pass dispatch input via env
…s wake

- re-evaluate status at the next boundary (work start, lunch, end, pause, vacation)
- never schedule a reminder past the current working stretch
- wake no longer restarts a still-valid countdown
- ignore snooze outside working time; log autoStart only when work starts
- clear delivered notifications so stale banners cannot flip posture
- flush history on quit and in tests instead of sleeping
- move totals out of MenuPopoverView into PostureStats, clipped to work
  hours minus lunch and never crossing midnight
- popover: TimelineView instead of a stored 1 Hz timer, pause state from status
- vacations cover whole days, single-day vacations allowed
- launch at login reads SMAppService, stand warning at 45 min
- custom mode no longer shows a preset as selected, single settings write
  for shared hours, hours validation hint, science text fix
@bendns
bendns marked this pull request as ready for review October 20, 2026 13:15
@bendns
bendns merged commit 37a1c34 into main Oct 20, 2026
1 check passed
@bendns
bendns deleted the fix/code-review-findings branch October 20, 2026 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant