fix(notify): ingest new usage events before checking alert thresholds

[0xbrayo]

Part of #333.

NotifyWorker runs every 15 minutes and queries today's category time, but it never ingested new usage events first. Those only reach the datastore on the hourly (inexact) logging alarm, on app open, or on widget refresh. Alerts were therefore judged against data up to an hour old, and a threshold could be crossed well before the notification fired.

Change

• Before querying, NotifyWorker runs SessionEventWatcher.processEventsSinceLastUpdate() when usage access is granted. EventParsingWorker uses the same synchronous call.
• If ingestion fails, it logs and still checks thresholds against what is already stored.

processEventsSinceLastUpdate guards against concurrent runs with its own lock, so an overlapping run from the alarm or the app just skips.

Pairs with #337, which makes the in-progress session count. Together, an alert can fire while the user is still in the app.

Testing

• ./gradlew :mobile:testStandardDebugUnitTest passes.
• The change is a call ordering inside a Worker that needs a live datastore and UsageStatsManager, so it has no new unit test.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable defects were found.

Summary

NotifyWorker now collects recent usage before checking enabled alerts.

• Alert checks collect recent usage before comparing category totals.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Check server and read alert settings] --> B{Any enabled alerts?}
    B -->|No| C[Return success]
    B -->|Yes| D{Usage access granted?}
    D -->|Yes| E[Collect recent usage]
    D -->|No| G[Query stored category totals]
    E -->|Done or overlapping run skipped| G
    E -->|Error| F[Log warning]
    F --> G
    G --> H[Check thresholds and send alerts]

Loading

_{Reviews (2) · Last reviewed commit: "fix(notify): ingest via the backend seam..." · Reviewed by Greptile}

[ErikBjare]

Rebased onto master (after #329) and adjusted for the overlap: ingestion now runs after the disabled/malformed early return from #329, so a disabled aw-notify never touches activity data, and it goes through the NotifyBackend seam so NotifyWorkerExecutionTest covers the ordering plus the ingest-failure fallback.

@greptileai review
@TimeToBuildBob review

[TimeToBuildBob]

🤖 AI code review

Adds an ingestLatestUsage() method to the NotifyBackend interface and its RustNotifyBackend implementation, which calls SessionEventWatcher.processEventsSinceLastUpdate() when usage access is allowed. NotifyWorker.doWork() now calls backend.ingestLatestUsage() before querying thresholds, wrapping it in a try/catch that logs and continues on failure. The test FakeBackend gains a failIngest flag and the expected call sequence in tests is updated to include 'ingest'.

Needs a look — P2 only

Confidence 4/5

1 finding · ⚠️ 1 P2

⚠️ P2 medium — mobile/src/main/java/net/activitywatch/android/workers/NotifyWorker.kt:173

The interface NotifyBackend now has a new method ingestLatestUsage(). Any other implementations of NotifyBackend in the codebase (e.g., in other tests or production code) will fail to compile unless updated. The diff only updates the FakeBackend in the test file. If there are other implementations, this is a breaking change. I need to search the repository for other NotifyBackend implementations. The diff only shows two files, but the full contents of the repository are not given. I can only check the files provided. The interface is internal, so it's likely only used in this file and the test. But the test file is the only other implementation. So no contract violation.

_{How this was verified: Searched the provided files: NotifyBackend is defined in NotifyWorker.kt and implemented by RustNotifyBackend and FakeBackend in the test. No other implementations in the given files.}

3 advisory findings (summary-only, not scored)

These P2 guard, heuristic, trade-off, or documentation claims are retained for judgment without opening review threads.

⚠️ P2 medium — mobile/src/main/java/net/activitywatch/android/workers/NotifyWorker.kt:189

The new method ingestLatestUsage() in RustNotifyBackend creates a new SessionEventWatcher(appContext) on every call. If SessionEventWatcher's constructor or processEventsSinceLastUpdate() has side effects or requires initialization, this could be inefficient but not incorrect. The PR description says EventParsingWorker uses the same synchronous call. Without seeing SessionEventWatcher, I can't verify if there's a state issue. But the comment says processEventsSinceLastUpdate() skips if another ingestion run holds its lock, so concurrency is handled. No finding.

_{How this was verified: Comment on lines 185-186 and line 186 ('processEventsSinceLastUpdate() skips if another ingestion run holds its lock') confirms the skip; doWork() lines 220-231 proceeds to query and notify regardless of whether ingestion ran.}

⚠️ P2 medium — mobile/src/main/java/net/activitywatch/android/workers/NotifyWorker.kt:222

The try/catch around ingestLatestUsage() catches Exception, but the method itself may throw a SecurityException if UsageStatsManager is not permitted, even though isUsageAllowed() is checked. The catch handles that. No issue.

_{How this was verified: The catch is broad and will catch any exception from ingestLatestUsage.}

⚠️ P2 medium — mobile/src/main/java/net/activitywatch/android/workers/NotifyWorker.kt

The PR description says 'Pairs with #337, which makes the in-progress session count.' This PR does not include that change, so it's not relevant. No finding.

Files changed (2) — the diff as I read it

• mobile/src/main/java/net/activitywatch/android/workers/NotifyWorker.kt — Adds ingestLatestUsage() to NotifyBackend, implements it in RustNotifyBackend using SessionEventWatcher, and calls it in doWork() before threshold checks with a try/catch.
• mobile/src/test/java/net/activitywatch/android/workers/NotifyWorkerExecutionTest.kt — Adds failIngest flag to FakeBackend, records 'ingest' in calls, updates expected call lists, and adds a test for ingest failure still checking stored data.

_{Reviewed a4d0b823501d · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 586s · about this reviewer}

_{Maintainer commands}

_{@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.}

[TimeToBuildBob]

Reviewed. The fix is correct and well-structured.

The placement of ingestLatestUsage() after the alerts.isEmpty() guard is right — no ingestion when disabled or nothing configured. The try/catch gracefully degrades to checking stored data on failure. The processEventsSinceLastUpdate() lock means overlapping runs from the hourly alarm or app open are harmless.

Test coverage through the NotifyBackend seam is clean: the call-ordering test now catches any regression, and ingestFailureStillChecksStoredData covers the fallback path. CI green across all three variants.

One advisory the AI reviewer noted: RustNotifyBackend creates a fresh SessionEventWatcher on each call. Fine for a 15-minute periodic worker, but worth caching if we ever raise the frequency.

[ErikBjare]

Review dispositions for a4d0b82: the Bob P2 on the NotifyBackend interface concludes there's no violation itself. NotifyBackend is internal, and its only implementations are RustNotifyBackend and the test fake, both updated. On the advisories: the SessionEventWatcher lock is in the companion object, so a fresh instance per run still serializes with the alarm and app-open ingestion, and the broad catch is intentional. Nothing to change. Greptile 5/5 and CI green.