feat(analytics): add permission-gated login-activity read api

[zaira-bibi]

What

Add a permission-gated read API for daily login activity — the first read-side slice of the analytics subsystem, built on the user.logged_in event.

Changes

• feat(migrations): add login_activity_daily TimescaleDB continuous aggregate (Postgres-only; no-op on SQLite)
• feat(core): declare and export the analytics.read permission
• feat(analytics): add dialect-aware get_login_activity read function, exposed via app.lib.analytics
• feat(api): add GET /api/v1/analytics/login-activity, gated by analytics.read
• feat(migrations): grant analytics.read to the seeded admin role
• docs: document analytics.read (README) and the read API (CLAUDE.md)

How to Test

1. make test.backend.pytest — all green (1316 passed), including tests/analytics/test_login_activity_read.py and the migration no-op guard in tests/analytics/test_analytics_migrations.py.
2. make mypy — clean; make lint.backend and make lint.format.backend check=1 — clean.
3. Single migration heads: uv run alembic heads and uv run alembic -c alembic_analytics.ini heads each show one head.
4. On real Postgres/TimescaleDB (make services.up, make migrations):

• make db-shell-analytics → \d+ login_activity_daily and SELECT * FROM timescaledb_information.continuous_aggregates; show the aggregate + refresh policy.
  • make db-shell → confirm the admin role has a analytics.read grant.
  • Log in as an admin and call GET /api/v1/analytics/login-activity?days=30; a non-admin gets 403.

Notes

• Migrations required (two lineages): app grant c5b6e911f9d6; analytics continuous aggregate 287f281e6558. Run make migrations.
• The continuous aggregate is TimescaleDB-only — a no-op on SQLite (tests/e2e), where the read path aggregates raw_events directly. The Postgres cagg and the admin grant can only be verified on real Postgres (step 4).
• No breaking changes, no new env vars, no dependency bumps.
• Frontend dashboard (the /dashboard/analytics page + typed client) is intentionally out of scope here and will follow in a separate PR.

This PR description was written with the assistance of an LLM (Claude).

[zaira-bibi]

@claude-review

[claude]

Claude finished @zaira-bibi's task in 2m 18s —— View job

---

Claude Code Review

• Read changed files & gather context
• Security & permissions review
• Correctness & logic review
• Migrations review
• Tests & docs review
• Post feedback

Reviewed against base origin/zaira/feat/analytics-plugin-event-schemas. This is a clean, well-scoped, well-documented PR — the read/write split, dialect-aware read path, permission gating, and the two-lineage migration story are all handled thoughtfully, and the test coverage is solid (buckets, days limit, permitted/forbidden/unauthenticated). Two non-blocking correctness/clarity points below; no security issues found.

✅ Strengths

• Layering is correct — read queries in app/core/analytics/reads.py, re-exported through app/lib/analytics, consumed by the API. No reaching into internals.
• Permission gating is textbook: analytics.read declared via Permission.create(), exported through app/lib/permissions, gated with require_in_global_scope(), granted to admin via an idempotent INSERT … SELECT migration. The now() literal-SQL pattern matches the parent migration 9477cf5a43a0, and keeping permission strings as literals in the migration (not imports) is the right call.
• Dialect-aware read path with a matching no-op guard on the cagg migration, plus a test (test_login_activity_cagg_migration_is_noop_on_sqlite) that proves the analytics lineage still upgrades to head on SQLite.
• Parameterized SQL (:days) — no injection surface. Query(ge=1, le=365) bounds the input.
• Docs (README + CLAUDE.md) and the regenerated frontend/lib/api/generated.ts were updated alongside the code.

🟡 Medium — days limits rows, not a calendar window

get_login_activity orders by day DESC and LIMIT :days, so days=30 returns up to 30 days that had ≥1 login — not "the last 30 calendar days." With gap days the series can reach back further than 30 calendar days, and there's no lower date bound. The endpoint docstring ("for the last days days") reinforces the calendar-window reading, so a dashboard labelled "last 30 days" could silently show older data. Also note days with zero logins are absent (no zero-fill) — the frontend must handle gaps. Either behaviour is fine, but pick one and make the naming + docstring match. Details inline on reads.py.

🟢 Low — make the UTC-bucketing assumption explicit

The comment says the PG and SQLite variants "must stay semantically identical," but time_bucket('1 day', occurred_at) (timestamptz → UTC buckets) vs date(occurred_at) (buckets on the stored offset) agree only while events are UTC. A one-line note that both bucket on UTC would guard against future divergence. Inline on reads.py.

Nits

• reads.py: session.bind.dialect.name assumes bind is non-None; fine at runtime for a request-scoped session and mypy is reported clean, so just flagging for awareness.

Overall

👍 Approvable once the days semantics (Medium) are confirmed as intended or adjusted — that's the only item I'd want resolved before merge; the rest are clarity nits. Nice work.
· zaira/feat/analytics-milestone-1

[claude]

[Medium] LIMIT :days caps the number of returned rows, not a calendar window.

Both variants order by day DESC and LIMIT :days, so days=30 returns up to 30 days that had at least one login — not "the last 30 calendar days." If there are gap days (days with zero logins), the result can reach back well beyond 30 calendar days, and there is no lower date bound. The endpoint docstring in analytics.py ("for the last days days") reinforces the calendar-window reading, so a dashboard labelled "last 30 days" may silently show older data.

Two consequences worth deciding on explicitly:

1. No date floor — consider adding WHERE occurred_at >= now() - (:days || ' days')::interval (PG) / date(occurred_at) >= date('now', '-' || :days || ' days') (SQLite) if a true window is intended, or rename/clarify to "most recent N data points" if row-capping is intended.
2. No zero-fill — days with no logins are simply absent from the series; the frontend must be prepared to handle gaps.

Either behaviour is defensible, but the naming (days) and docstring should match the actual semantics.

[claude]

[Low] Make the UTC-bucketing assumption explicit. The two variants "must stay semantically identical," but they bucket differently: PG's time_bucket('1 day', occurred_at) on a timestamptz column buckets in UTC, while SQLite's date(occurred_at) buckets on whatever offset the stored string carries. They agree only as long as events are stored/interpreted as UTC (which the current ingest path does). A one-line note here that day boundaries are UTC on both paths would prevent a future timezone-parameterization on the PG side from silently diverging from SQLite.

[abdulrafey1]

Lets avoid adding raw SQL in code

[zaira-bibi]

went with raw SQL here on purpose as the postgres query reads the login_activity_daily continuous aggregate, which isn't (and shouldn't be) registered on analytics_metadata, reason being that the autogenerate will try to create/drop it and it doesn't exist on sqlite either. also worth noting that the two can't share a query (because of different source relation and day bucketing), so it's two selects either way.

the sqlite query could be written in sqlAlchemy core, but then the readers couldn't diff them by eye.

[abdulrafey1]

Could we check if we can do this via the ORM

[zaira-bibi]

unfortunately sqlAlchemy does not have DDL construct for materialized views at all.

[abdulrafey1]

Same here

[hamza-56]

This start_offset combined with WITH NO DATA means anything older than 3 days at migration time never gets materialized: once the first policy run advances the watermark, real-time aggregation stops covering those old buckets and they disappear from the view entirely. Since user.logged_in has been emitting for a while, a deployed instance silently loses its pre-migration history here (the SQLite path keeps full history, so the two dialects drift). The comment above mentions an out-of-band backfill, but nothing in the repo runs one. Creating the view WITH DATA, or a one-off refresh_continuous_aggregate(login_activity_daily, NULL, NULL), would fix it.

[zaira-bibi]

we'd need a refresh_continuous_aggregate(xyz_activity, NULL, NULL) for every cagg that's created, which is the wrong shape. i can either do (a) start_offset => NULL in the policy itself to refresh from the beginning of time or (b) one generic backfill command to discover all caggs and full-refresh each one. i lean towards (b) as (a) will cause each refresh to re-scan full history, which will grow expensive as data accumulates. what do you say?

[hamza-56]

time_bucket buckets on UTC midnight, but to_char renders a timestamptz in the session TimeZone, and we don't pin one on the analytics engine. On any server west of UTC every label shifts one day back, so today's logins show up under yesterday. The SQLite path is effectively UTC, so tests will never catch this. to_char(day AT TIME ZONE 'UTC', 'YYYY-MM-DD') makes it deterministic.

[hamza-56]

LIMIT :days returns the N most recent days that had logins, not "the last N days" as the endpoint doc says, so a quiet stretch pushes the window arbitrarily far back. It also leaves this SQLite branch scanning all of raw_events on every call. A WHERE occurred_at >= ... bound on both queries would match the documented contract and cap the scan.