Skip to content

Test Notification Thresholds through tick() behind an injected notification seam - #41

Merged
64x-lunicorn merged 9 commits into
mainfrom
refactor/notification-threshold-seam
Sep 15, 2026
Merged

Test Notification Thresholds through tick() behind an injected notification seam#41
64x-lunicorn merged 9 commits into
mainfrom
refactor/notification-threshold-seam

Conversation

@64x-lunicorn

@64x-lunicorn 64x-lunicorn commented Sep 14, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

Makes the Notification Threshold policy testable through WorkdayManager's interface and fixes how thresholds are reported when Net Work Time jumps past several at once.

  • Notification seam (ADR-0001 pattern): WorkdayManager sends threshold and new-day notifications through an injected WorkdayNotificationSending, defaulting to NotificationManager.shared. Tests use a recording adapter.
  • Pinned behaviour through tick(): tests move the ManualClock forward and call tick() to cover:
    • the milestone, including the ArbZG end-of-day prompt when notifications are disabled;
    • normal and critical;
    • the recent-activity and pending-Idle-Period guards;
    • persistence across reloads.
  • Behaviour change: only the highest crossed Threshold is reported.
    • The problem: the check pauses during inactivity, sleep and lock, so Net Work Time can jump past several Thresholds. Previously the lower ones were then caught up one tick at a time in reverse order: milestone, then critical, then normal.
    • Now: only the highest is sent, and the lower ones already reached are recorded as notified.
  • Named type: notified Thresholds are a NotifiedThreshold type instead of free strings. The Daily Log JSON is unchanged, old logs decode, and unknown values are preserved.
  • Housekeeping: shared recording prompt adapter and notification test fixture, and .scratch/ added to .gitignore.

How to test?

  • Automated: xcodebuild test -scheme OpenWorktimeTrackerTests runs 282 tests. The new notification test classes all pass.
    • Known failure (also on main at b6aa5e5): ViewLayoutTests.testSettingsRowsRemainReadableInBothLanguagesAndAppearances fails with 7 OCR assertions.
  • Manual:
    • Run the app and confirm the new-day and threshold notifications still appear.
    • Set low thresholds, stay idle past several of them, then come back. Expect exactly one notification, for the highest Threshold crossed.

Open question

If normal or critical is configured above the milestone (the settings allow it), the milestone does not suppress them; they still fire when reached.

Checklist

  • Tests pass (make test): everything except the OCR test above, which also fails on main
  • Linting passes (make lint): 0 serious, same 5 length warnings as main
  • No new external dependencies added
  • Uses @Observable (not ObservableObject)
  • Uses DesignTokens for colors/typography (no UI changes)

🤖 Generated with Claude Code

https://claude.ai/code/session_01WFc8P7zoT4V7GQaqwegkcm

Lunicorn-lab and others added 9 commits September 14, 2026 21:09
@Lunicorn-lab @claude
refactor: send notifications through an injected seam
63d586c 
WorkdayManager no longer reaches for NotificationManager.shared. It takes a
WorkdayNotificationSending dependency through init, defaulting to the
production adapter, the same way it receives its prompt presenter. The
new-day notice is pinned through a recording test adapter.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFc8P7zoT4V7GQaqwegkcm
@Lunicorn-lab @claude
test: pin milestone Notification Threshold through tick()
0ee66f8 
Describe the milestone through WorkdayManager's interface only: it notifies
once when enabled, still prompts to end the day when notifications are
disabled, is recorded in the Daily Log across a reload, yields to a pause or
a pending Idle Period before the prompt appears, and waits for recent
activity. The recording prompt adapter is shared so it can record the
end-of-day prompt.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFc8P7zoT4V7GQaqwegkcm
@Lunicorn-lab @claude
test: pin normal and critical Notification Thresholds through tick()
fce8211 
Normal and critical each notify once, stay silent when notifications are
disabled, and survive a reload from the same store. Pin the agreed edge
cases where Net Work Time jumps past several Thresholds at once: critical is
notified instead of normal, and critical follows the milestone one tick
later.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFc8P7zoT4V7GQaqwegkcm
@Lunicorn-lab @claude
refactor: record notified Thresholds as a named type
10ba348 
A Workday's notified Notification Thresholds are a Set<NotificationThreshold>
instead of free strings that only matched the notification identifiers by
coincidence; ThresholdType now derives its identifier from the same type.
NotificationThreshold is string-backed, so Daily Logs encode the same strings
as before, previous Daily Logs still count as already notified, and an
unknown value is kept rather than making the file unreadable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFc8P7zoT4V7GQaqwegkcm
@Lunicorn-lab @claude
fix: report only the highest crossed Notification Threshold
7625fd3 
When Net Work Time jumps past several Notification Thresholds at once, only
the highest one is notified. The lower Thresholds crossed in the same jump
are recorded as notified, so they no longer follow one tick later or after a
reload. The milestone records them even when notifications are disabled.
Crossing Thresholds one after another still notifies each once.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFc8P7zoT4V7GQaqwegkcm
@Lunicorn-lab @claude
refactor: address review of the Notification Threshold seam
905ae86 
- Rename NotificationThreshold to NotifiedThreshold so it no longer reads
  as the configured NotificationThresholds.
- The WorkdayNotificationSending seam speaks in NotifiedThreshold plus
  hours; NotificationManager.ThresholdType is gone, leaving one list of
  Threshold kinds. The recording adapter records the typed kind.
- Notification tests share one fixture with recording prompts, an isolated
  widget store and main-queue draining, and release their manager in
  tearDown. The previous-version Daily Log test moves next to the other
  threshold tests.
- The disabled-notifications test gains a positive control, and small
  wording and decoder duplication fixes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFc8P7zoT4V7GQaqwegkcm
@Lunicorn-lab @claude
chore: ignore local .scratch tickets
5b4c701 
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFc8P7zoT4V7GQaqwegkcm
@64x-lunicorn
Merge branch 'main' into refactor/notification-threshold-seam
b546cb2
@Lunicorn-lab @claude
Merge main into refactor/notification-threshold-seam
1752960 
Resolve the XcodeGen project conflict by regenerating it from project.yml,
so it contains both the shared Workday appearance files and the
notification seam files.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFc8P7zoT4V7GQaqwegkcm
@64x-lunicorn
64x-lunicorn merged commit d1317e7 into main Sep 15, 2026
2 checks passed
@64x-lunicorn
64x-lunicorn deleted the refactor/notification-threshold-seam branch September 15, 2026 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@64x-lunicorn @Lunicorn-lab