feat(activity-tracker): create a storage module to track activities in multiple application

[Sam1kumar]

No description provided.

[Vipin9821]

⚠️ Request changes — PR #44 (feat(activity-tracker): core storage module) — DEEP review

Stats: +2417 / −44, 17 files · Scope: The core activity-tracking module — session lifecycle (track/logEvent/startSession), pause/resume for backgrounding, Realm-backed buffer, upload sync, process-wide singleton.

Files deeply reviewed (every line): activity_repository.dart (full abstract — excellent API design), activity_repository_impl.dart (full — all lifecycle methods, serialized pattern, sync, clear), activity_dao_impl.dart (full — Realm DAO, buffer trimming), activity_session.dart (model).

⚠️ Impact on Existing Functionality

✅ New module — additive. No existing code touched. This is the foundation for GT #4449 and FA-One #78.

---

🟡 Medium

1. ZERO TESTS for a core multi-app module

This is the foundation for two production apps (GT + FA-One). The following critical logic is entirely untested:

• _serialized mutation queue (ordering, error propagation, lock chaining)
• track → isSameScreenAs → continue-or-switch decision
• pauseCurrentSession / resumeLastSession lifecycle (stash, discard-if-current, resume-as-new)
• _rollOverCurrentSession (close + buffer + reopen identical)
• sync failure path (buffer preserved on upload error)
• ActivityDaoImpl._trimTo (buffer overflow: oldest dropped, newest kept)
• _readSession malformed-JSON drop

Per the skill's matrix, a new module with this much business logic and concurrency = ✅ Blocking test category.

Action: Add tests for at minimum: session lifecycle (start→end), same-screen continuation, pause/resume round-trip, sync failure preserves buffer, buffer trim at 2000+.

2. savePendingActivities non-atomic replace

deleteAll then addAll — if the app crashes between, all pending activities are lost. addPendingActivity (singular) is the safe path. The bulk-replace is riskier.

Action: Consider Realm's transaction to make the delete+add atomic, or use upsert semantics.

---

🟢 What's done well (exceptional)

• track vs logEvent vs startSession — the doc explains the distinction perfectly: "track would close the session for the screen they are on and open one for the tap, which then stays open until they navigate, attributing all the intervening time to a button press." This is deep UX understanding baked into the API.
• instanceOrNull — the GT app's instanceOrNull?.track() pattern works because of this. Tracking absent → silent no-op, zero crash risk.
• _serialized queue — clean mutex pattern; errors don't block subsequent operations; logEvent deliberately outside the lock (with reasoning).
• _rollOverCurrentSession — "a user parked on a single screen for hours would report nothing until they finally navigated away" — solved by close-buffer-reopen. Smart.
• Buffer overflow protection — maxPendingActivities = 2000 with trim-oldest-first. Documented.
• _readSession drops malformed — "would otherwise wedge tracking permanently" — defensive and correct.
• Pause/resume with stash-discard-if-current — handles the race where a new session starts during the pause.

📋 Action Checklist

• P1 Add tests for session lifecycle, pause/resume, sync failure, buffer trim (#1)
• P2 Make savePendingActivities atomic (#2)

Overall: Genuinely excellent module design — the API surface, concurrency model, and defensive patterns are among the best I've reviewed. The blocking gap is the absence of tests for a module this critical. Add lifecycle + concurrency tests, then this is an approve.

/cc @Sam1kumar