diff --git a/UI-EVENTS.md b/UI-EVENTS.md new file mode 100644 index 0000000..e968c82 --- /dev/null +++ b/UI-EVENTS.md @@ -0,0 +1,380 @@ +# UI product-event instrumentation — design + +**Status: design + prototype. Feature-flagged OFF. Not deployed anywhere.** +Enabling it in any environment requires the operator decisions in +[§6](#6-decisions-needed-from-the-operator) first. + +The goal is to learn what people actually *do* in the Thermograph UI — did they +ever get a location, does search work, which views earn their maintenance — with +instrumentation that is *structurally incapable* of tracking a person, so it +needs no consent banner and contradicts nothing on the public privacy page. + +--- + +## 0. What already exists (build on this, don't reinvent) + +| Seam | What it does | Where | +|---|---|---| +| Request middleware | classifies + counts every inbound request, writes an access JSONL line **including the raw client IP** | `backend/web/app.py` `revalidate_static` | +| `core/metrics.py` | since-start counters (inbound by category, outbound by source, heartbeats) behind `GET /api/v2/metrics` | `backend/core/metrics.py` | +| **Product-event beacon** | `POST /api/v2/event`, `navigator.sendBeacon`, allowlisted names, per-IP rate limit, referrer reduced to a bare domain, always 204 | `api_event` + `metrics.record_event` | +| Event counters | `(event, referrer_domain, UTC day)` → count, **7-day** window, UNLOGGED, TRUNCATEd every deploy | `metrics.EVENTS`, `event_cume` | +| Client tracker | `track(event)` + `[data-event]` click delegation | `frontend/static/digest.js` | +| Activity JSONL | `audit.log_activity(tag, record)` → `logs/activity/*.jsonl` | `backend/core/audit.py` | +| Loki/Grafana | Alloy ships `/applogs/**/*.jsonl` fleet-wide, **30-day** retention | `observability/` | + +So four coarse events (`home.locate`, `home.digest_signup`, `home.records_click`, +`home.share`, plus `home.nav_*`) are already live, on the SSR homepage only. + +**Three gaps this design closes.** + +1. **No dimensions.** `home.locate` is a bare counter. It cannot say whether the + tap *succeeded*, which is the only interesting part. +2. **No durability.** The counters are UNLOGGED and TRUNCATEd on every deploy, + and capped at 7 days. Loki holds 30 days of JSONL. Neither survives a quarter, + so no seasonal or before/after comparison is possible — which is most of the + value for a site about *weather*. +3. **Nothing outside the homepage.** Calendar, Day, Compare, Score, Alerts — the + whole interactive product — is uninstrumented. + +Two corrections to assumptions that were made when this work was scoped: +Loki retention is **30 days** (`observability/loki/config.yml`, `720h`), not 24h; +and the Alloy glob `/applogs/**/*.jsonl` already picks up the `activity/` stream, +so a new JSONL sink needs no shipping config. + +--- + +## 1. The events worth capturing + +Traffic is ~9,700 req/day of which ~60% is crawlers, so **~3,000 human req/day**. +At that volume a firehose buys nothing a well-chosen aggregate doesn't, and every +extra event is another thing to keep honest. Seven events, each tied to a decision +someone would actually make: + +| Event | Question it answers | Decision it informs | +|---|---|---| +| `view.open` | How many sessions reach each view, and from where? | The denominator for everything else; whether Compare/Score justify their maintenance | +| `place.pick` | Does anyone ever get past "no place picked", and by which route (geolocate / map / search / restore / link)? | **The activation question.** If geolocate is mostly declined, promote the map picker | +| `place.search` | Does location search find anything? (`hit` / `typo` / `miss`) | Whether to invest in the geocoder/typo tolerance | +| `view.control` | Which in-view controls get used — chart metric, °F/°C, date, range, season, compare add/remove, distribution metric | Which chart metrics to keep/promote; whether the unit default is wrong | +| `share` | Is the permalink / PNG export used at all? | Whether to keep maintaining the SVG→PNG exporter | +| `alert` | The alerts funnel: open → start → create, push on/off | Where subscription signup leaks | +| `deadend` | Errors the visitor actually *saw*: grade error, no data, geolocation denied, search miss, 404, offline | Bug reports nobody filed | + +**Deliberately not captured** — because the request log already answers them, and +duplicating a signal means two numbers that will eventually disagree: page-view +counts, API latency/error rates, and asset traffic. Also deliberately not +captured: hover/scroll/dwell (needs a session identifier to mean anything), +map pan/zoom (very high volume, low decision value, and pan/zoom coordinates +are location data), and anything at all about *which* place was chosen. + +### Two events are recorded server-side, not in the browser + +`place.search` is recorded in `api_suggest`: the server already knows the answer, +every surface that searches is covered for free, and **the query string never has +to leave the request handler** — only the three-way outcome is stored. Same +argument would apply to alert creation (`accounts/api_accounts.py`). Prefer this +whenever the server can derive the fact: less client code, no forgeability, and +strictly less data in flight. + +--- + +## 2. Schema + +Small, typed, and *closed*. Three dimension slots (`view`, `prop`, `value`) plus +a referrer domain on `view.open` only. Every value is an enum from a fixed +allowlist (`backend/core/events.py` `SCHEMA`). Unknown event name → one `other` +bucket with blank dimensions; unknown value in a declared slot → `other`; a slot +an event doesn't declare → dropped entirely. **Nothing a client sends is ever +persisted verbatim.** + +``` +view.open view= prop=direct|nav|link|restore +referrer domain +place.pick view= prop=geolocate|map|search|restore|link +place.search prop=hit|typo|miss +view.control view= prop= value= +share view= prop=link|png|permalink +alert prop=open|start|create|push_on|push_off|test value=y|n +deadend view= prop=grade_error|no_data|geo_denied|search_miss|not_found|offline +``` + +`VIEWS` = home, calendar, day, compare, score, alerts, legend, city, cities, +records, month, glossary, about, privacy. +`CONTROLS` = chart_metric, unit, date, range, season, totals, dist_metric, +comfort, compare_add, compare_remove, filter_sheet, day_step. +`VALUES` = the metric keys, `F`/`C`, `today`/`past`/`future`, count buckets +(`1`…`5plus`), span buckets (`1y`…`max`), season keys, `on`/`off`, `other`. + +Numbers are bucketed **client-side** before they are sent (`5plus`, not `7`): +a raw count is a small amount of entropy, and a bucket is all the decision needs. + +**Storage row:** `(hour_bucket, event, view, prop, value, referrer) → count`. +There is no row per interaction, and no column an identifier could go in. +Worst-case key space is a few hundred rows/hour; realistically ~50. + +Migration note: the four live `home.*` events keep their current unflagged, +un-dimensioned behaviour so the ops endpoint's numbers don't reset when this +lands. Fold them into `view.open`/`place.pick`/`share` and retire the old names +as a follow-up, once there's overlap data to stitch the series. + +--- + +## 3. Transport and storage + +**Chosen: the existing beacon → two sinks.** + +* **Transport** — extend `POST /api/v2/event` (`navigator.sendBeacon`, batched, + always 204). Already built, already excluded from the traffic counters, already + rate-limited. Batching (≤20 events, flushed on a 4 s timer and unconditionally + on `pagehide`/`visibilitychange`) matters because chart-metric taps and slider + drags arrive in bursts. +* **Durable sink** — `ui_event_hourly`, a TimescaleDB hypertable + (`backend/alembic/versions/0003_ui_events.py`): 30-day chunks, a native + retention policy, and an `ON CONFLICT … count + 1` upsert. The DB is already + TimescaleDB with hypertables and a compression/retention pattern to copy. +* **Live sink** — one `audit.log_activity("ui.event", …)` JSONL line per event. + Alloy already ships that path, so the last 30 days are queryable in Grafana + immediately without waiting on the rollup. Same allowlisted enums, nothing else. +* **The existing in-process counters stay untouched** — they remain the live + "what's happening right now" view on `/api/v2/metrics`. + +**Why not the alternatives:** + +* *Loki only.* 30-day retention loses every seasonal comparison — the thing a + weather product most wants. Loki is also a poor aggregation engine for + "unit switches per week over a year". Kept as the live tier, rejected as the + system of record. +* *Extend the in-process counters.* Cheapest option and no new storage, but they + are UNLOGGED, TRUNCATEd every deploy, capped at 7 days, and have exactly one + dimension. Every question in §1 needs at least two. Kept, not extended. +* *Per-event rows in a hypertable (a real firehose).* With no identifier, a row + per interaction gives you nothing an hourly aggregate doesn't — except + intra-hour timing, which no listed decision needs. It costs a table that + *could* hold a behavioural sequence, which is exactly the property that would + make the privacy analysis hard. Rejected on privacy grounds, not cost. +* *A third-party analytics SaaS (Plausible/Umami/GA).* GA is a non-starter + (US transfer, consent banner, cookies). Self-hosted Plausible/Umami would be + defensible, but it means another service to run and its own identifier model + (Plausible hashes IP+UA+salt — a pseudonymous identifier, which is precisely + the decision being deferred to the operator in §6.1). Rejected as more moving + parts for less control. + +--- + +## 4. Abuse and bot resistance + +The endpoint is public and unauthenticated on a site where 60% of traffic is +crawlers and scanners. Layered, in order of how much they actually buy: + +1. **Closed allowlist** (the real defence). Nothing outside `SCHEMA` is stored; + unknown names collapse into one `other` bucket. Storage is bounded by the + *product of the enums*, not by what anyone sends. The endpoint cannot be grown + into a spam sink or a data-exfil channel. +2. **Body cap, enforced while streaming** — `MAX_BODY_BYTES = 4096`; a declared + `Content-Length` past it is refused without reading, and an undeclared one + stops mid-stream. (The cap must comfortably exceed a full 20-event batch, or + the byte cap silently eats legitimate traffic instead of abusive traffic — + there is a regression test for exactly that.) +3. **Per-IP rate limit** — the existing token bucket, raised from 30/min to + **120/min** (`THERMOGRAPH_EVENT_RATE_PER_MIN`) because 30 was sized for four + coarse events and a batched interaction stream would silently truncate normal + use, biased toward the most engaged visitors. The IP is a key in a map cleared + every minute; it is never written anywhere. +4. **Soft first-party check** — `Sec-Fetch-Site` must be `same-origin`/ + `same-site`/`none`; a `cross-site` value is dropped, and a POST with neither + `Sec-Fetch-Site` nor `Origin` is flagged as the first bucket to look at. + Every browser that can run our JS sets this; naive scanners don't. Explicitly + **not** a security boundary — anything that bothers can spoof it. +5. **Uniform 204** — counted, rejected, over-cap, and rate-limited are + indistinguishable, so probing tells an attacker nothing. +6. **No UA-based bot filtering.** Blocking on User-Agent is a coin flip and gives + false confidence. The structural defence is that the beacon requires JS + execution and is linked from nowhere. + +**Poisoning** (someone inflating a number rather than exhausting storage) cannot +be fully prevented without identity. Mitigation is cross-checking: the server's +own inbound page counts are an independent measure of `view.open`, so a +divergence between them is the alarm. Any analysis should treat these numbers as +*directional*, never as revenue-grade. + +--- + +## 5. Privacy position + +The site is EU-hosted with EU visitors, so GDPR + ePrivacy apply. The public +privacy page currently promises, verbatim: *"no analytics library, no cookies for +tracking, and no per-visitor identifier… simple aggregate counts… with nothing +tying any of it to an individual."* That is a commitment already made, and this +design is built to stay inside it rather than to renegotiate it. + +**What the design does:** + +* **No identifier of any kind.** No cookie, no `localStorage`/`sessionStorage` + id, no fingerprint, no hashed IP+UA. Not even the logged-in user id: the + handler never reads the session cookie, and `sendBeacon` cannot attach + credentials cross-origin anyway. +* **Nothing is written to the visitor's device**, so ePrivacy Art. 5(3) + (the "cookie rule", which covers *any* storage or access on terminal equipment, + not just cookies) is not engaged at all. +* **No IP is stored** by this feature — used as a per-minute rate-limit key and + discarded. +* **No free text, no coordinates, no URLs.** `place.pick` records *how* a + location was chosen, never where. The referrer is reduced server-side to a bare + registrable domain, never a full URL (which can carry someone's search query). +* **Aggregate-only storage.** An hour bucket and a tuple of enums; no row per + interaction, so no behavioural sequence exists to be reconstructed. +* **GPC and DNT are honoured** — a visitor signalling either sends nothing. + +**Consequence to be honest about:** with no identifier there are **no unique +visitors, no sessions, no funnels, no bounce rate, no returning-visitor rate**. +`view.open` counts view opens, not people. Ratios across events +(`place.pick / view.open`) are *rates*, not *conversion*, because the numerator +and denominator can't be tied to the same visit. That is the price of not needing +consent, and it is worth it — but it must not be quietly forgotten when someone +later asks "how many users do we have?" + +**Legal reading (for the operator to confirm, not for me to decide):** with no +identifier, no device storage, no cross-site data and aggregate-only retention, +this is anonymous audience measurement — it fits the CNIL exemption criteria and +the EDPB's "strictly necessary"/first-party-analytics reasoning, so **no consent +banner should be required**. Transient IP processing for rate limiting rests on +legitimate interest (Art. 6(1)(f)) and is security-necessary. No DPIA is +triggered (no systematic monitoring, no profiling, no special categories). One +line should be added to the Art. 30 record of processing. + +**A pre-existing issue this surfaced, unrelated to the new events.** +`audit.log_access` writes the **raw client IP** for every non-static request into +`logs/access/*.jsonl`, which Alloy ships to Loki with 30-day retention. That is +personal data under GDPR, it is retained for 30 days fleet-wide, and it sits +awkwardly beside the privacy page's *"not logged beyond the normal web-server +request handling"*. **This design does not touch it and does not extend it** — +but it should be decided on (§6.3) rather than left implicit, and it would be +odd to ship privacy-first analytics while the access log keeps raw IPs for a +month. + +--- + +## 6. Decisions needed from the operator + +Nothing below is decided in the prototype. The flag stays off until these are +answered. + +**6.1 — Identifier: none, ephemeral session, or per-user?** +Recommended: **none** (what the prototype implements). +*Consequence of "none":* no unique visitors, no funnels, no retention cohorts — +counts of interactions, not of people. +*Ephemeral session id* (random, `sessionStorage`, 30 min): enables funnels and +sessions. But it is storage on the visitor's device → **ePrivacy consent banner +required**, and it makes the data pseudonymous rather than anonymous +(subject-access/erasure duties attach). Also requires rewriting the privacy page. +*Per-user when logged in:* directly links behaviour to an identified person. +Highest analytical value, highest duty, and hardest to justify for a free weather +site. Recommended answer: **never**, regardless of what is chosen for anonymous +visitors. + +**6.2 — Failed search queries: capture the text or not?** +The single most useful piece of product data here is *what people searched for +when we found nothing*. Three options: (a) never capture — prototype default; +(b) capture **zero-result queries only**, server-side, normalised (lowercased, +length-capped, rejected if it contains `@`, a digit run, or a URL-ish token), +30-day retention, never joined to anything; (c) capture all queries — not +recommended, since successful queries are already answered by the `hit` counter +and a search box is a free-text field a person can type anything into. +Recommended: **(b), as a separately flagged follow-up**, with its own privacy-page +sentence. Do not bundle it with the initial rollout. + +**6.3 — Raw IPs in the access log (pre-existing).** +Keep as-is / truncate to /24 (IPv4) and /48 (IPv6) / hash with a daily-rotating +salt / drop entirely. Recommended: **truncate**, which preserves the geo-scale and +abuse-pattern uses while stopping single-device identification. This is +independent of the new events but should be decided in the same pass. + +**6.4 — Consent banner: rely on the analytics exemption, or add one?** +Recommended: **rely on the exemption** — it is only available while 6.1 stays +"none" and 6.2 stays "(a) or (b)". Adding a banner unlocks identifiers but costs +a real chunk of the (small) traffic to banner fatigue and adds a consent-state +machine to every page. + +**6.5 — Retention for `ui_event_hourly`.** +Set in the migration (`RETENTION_DAYS`, currently **400** — a full year-over-year +comparison). Anonymous aggregates make this a storage/usefulness question rather +than a GDPR one, but it should be a deliberate number. Options: 90 / 400 / +indefinite. + +**6.6 — Privacy-page copy.** +The page must keep matching reality *before* the flag goes on: the Analytics +section needs a sentence naming what is counted and stating the retention. Who +writes and approves that copy? + +**6.7 — Environment separation.** +Dev and beta run the same code. Should events be recorded there at all, and if +so, tagged with the environment so they never mix into prod's numbers? (A +`host`/`env` label already exists in Loki; the hypertable has no such column.) +Recommended: **flag on for dev only during validation, then off; prod-only after.** + +**6.8 — Referrer beyond `view.open`.** +The prototype keeps a referrer domain only on `view.open`. Widening it to every +event would allow "which referral source uses Compare" at the cost of a much +larger key space. Recommended: **leave as-is** until a question needs it. + +**6.9 — GPC/DNT.** +The prototype honours both, biasing counts slightly downward. Confirm that trade, +or drop it. + +--- + +## 7. Implementation plan + +### Backend (`backend/`) + +| File | Change | Status in prototype | +|---|---|---| +| `core/events.py` | **new** — `SCHEMA`, `normalize()`, hourly upsert, JSONL sink, `ENABLED` flag | done | +| `core/metrics.py` | expose `rate_ok()` so one event spends one token across both sinks; `record_event(rate_limit=False)`; raise the per-IP ceiling to 120/min and make it env-tunable; `reset_rate_limiter()` for tests | done | +| `web/app.py` | `api_event`: batched payload, streaming body cap, `Sec-Fetch-Site` check, fan-out to both tiers, still always 204. `api_suggest`: record `place.search` server-side | done | +| `alembic/versions/0003_ui_events.py` | **new** — `ui_event_hourly` hypertable + retention policy, Postgres-only (no-ops on the SQLite test bind, like `0002`) | done | +| `accounts/api_accounts.py` | record `alert` steps server-side on subscription create/delete | **todo** | +| `web/app.py` / `frontend/content.py` | record `deadend prop=not_found` on 404 responses | **todo** | +| `tests/core/test_events.py` | **new** — allowlist, slot dropping, no-free-text-by-construction, flag-off inertness, never-raises | done (10 tests) | +| `tests/web/test_event_beacon.py` | **new** — uniform 204, batch cap, oversize drop, cross-site drop, coordinates/free text never survive, legacy shape still counts | done (11 tests) | + +### Frontend (`frontend/`) + +| File | Change | Status | +|---|---|---| +| `static/track.js` | **new** — flag check, GPC/DNT opt-out, batching + flush on `pagehide`/`visibilitychange`, `currentView()`, `[data-event]` delegation, `trackLegacy()` for the four live events | done | +| `static/digest.js` | beacon moved out; re-exports `track`/`trackLegacy` so importers keep working | done | +| `static/app.js` | `view.open`, `place.pick` (geolocate + picker), `deadend` (geo denied, grade error, offline), `share` (link + png), `view.control chart_metric` | done | +| `static/mappicker.js` | thread a `method` ("search" vs "map") through `finish()` → `onPick(lat, lon, method)` | done | +| `static/units.js` | declarative `data-event` attributes on the °F/°C toggle — no import needed | done | +| `static/*.html` (6 SPA shells) | load `track.js` | done | +| `templates/base.html.j2` | stamp `data-tg-events` on `` when enabled | done | +| `content.py`, `app.py` | `EVENTS_ENABLED`, applied to both HTML paths (Jinja + memoized static shells) | done | +| `static/calendar.js`, `compare.js`, `day.js`, `score.js`, `subscriptions.js` | `view.open` + their own `view.control` / `place.pick` / `alert` call sites | **todo** | +| `templates/privacy.html.j2` | Analytics-section copy update — **blocked on 6.6** | **todo** | +| `tests/unit/test_pages.py` | flag absent by default on every HTML path; stamped on both paths when enabled | done (2 tests) | + +### Rollout + +1. Land with `THERMOGRAPH_EVENTS` **unset everywhere**. The feature is inert: + the beacon still answers 204, nothing reaches a sink, and the flag stamp is + absent from every page (asserted by test). +2. Resolve §6. Update the privacy page. Nothing turns on before this. +3. Run the migration on dev; enable on **dev only**; verify in Grafana + (`tag="ui.event"`) that the shape is right and that no free text or coordinate + ever appears. +4. Enable on beta for a week. Compare `view.open` against the middleware's own + page counts — a large divergence means bot noise or a client bug, and is the + go/no-go for prod. +5. Enable on prod. Re-check the `other` bucket and the `Sec-Fetch-Site`-less + bucket weekly for the first month; a growing `other` means either a schema gap + or someone poking the endpoint. +6. Kill switch: unset the flag. No deploy, no migration, no data loss — existing + rows just stop growing. + +### Cost check + +~3,000 human req/day → on the order of 1–3k events/day, batched ≈ a few hundred +extra requests/day (~0.005 req/s) and a handful of upserts. Negligible against a +48 GB box; the hypertable grows by roughly a few hundred rows/day, well under +100 MB/year before compression. diff --git a/backend/alembic/versions/0003_ui_events.py b/backend/alembic/versions/0003_ui_events.py new file mode 100644 index 0000000..a69d94b --- /dev/null +++ b/backend/alembic/versions/0003_ui_events.py @@ -0,0 +1,69 @@ +"""UI product events: hourly aggregate hypertable + +One row per (hour, event, view, prop, value, referrer) — never one row per +interaction. See ``backend/core/events.py`` for the schema and the reasoning: +the table has no column an identifier could be written into, so it cannot hold +a sequence of one person's actions even in principle. + +TimescaleDB hypertable on ``bucket`` with a 30-day chunk interval (the read +pattern is "last N weeks, grouped by event"), plus a retention policy so old +buckets drop themselves. ``RETENTION_DAYS`` is the operator-set retention limit +— change it here, not by hand on the box, so the deployed value is reviewable. + +Postgres-only: guarded to no-op on the SQLite bind (tests / CI / offline +Alembic), where events.py's aggregate sink is inert and only the JSONL activity +line is written. + +Revision ID: 0003_ui_events +Revises: 0002_climate_hypertables +Create Date: 2026-07-23 +""" +from alembic import op + +revision = "0003_ui_events" +down_revision = "0002_climate_hypertables" +branch_labels = None +depends_on = None + +# Aggregate rows carry no personal data, so this is a storage/usefulness choice +# rather than a GDPR one. 400 days keeps a full year-over-year comparison. +RETENTION_DAYS = 400 + + +def upgrade() -> None: + if op.get_bind().dialect.name != "postgresql": + return # SQLite fallback (tests/CI): the aggregate sink is a no-op there. + + op.execute("CREATE EXTENSION IF NOT EXISTS timescaledb") + + # Every dimension is NOT NULL DEFAULT '' rather than nullable: '' is a real + # value ("this event does not use this slot"), and it keeps the primary key + # usable (NULLs would defeat the ON CONFLICT upsert). + op.execute(""" + CREATE TABLE ui_event_hourly ( + bucket TIMESTAMPTZ NOT NULL, + event TEXT NOT NULL, + view TEXT NOT NULL DEFAULT '', + prop TEXT NOT NULL DEFAULT '', + value TEXT NOT NULL DEFAULT '', + referrer TEXT NOT NULL DEFAULT '', + count BIGINT NOT NULL DEFAULT 0, + PRIMARY KEY (bucket, event, view, prop, value, referrer) + ) + """) + # The PK includes the range-partition column (bucket), which TimescaleDB + # requires of any unique index on a hypertable. + op.execute( + "SELECT create_hypertable('ui_event_hourly', " + "by_range('bucket', INTERVAL '30 days'))") + op.execute( + f"SELECT add_retention_policy('ui_event_hourly', INTERVAL '{RETENTION_DAYS} days')") + + # The dashboard query is "one event over time", so index it that way. + op.execute("CREATE INDEX ui_event_hourly_event_idx ON ui_event_hourly (event, bucket DESC)") + + +def downgrade() -> None: + if op.get_bind().dialect.name != "postgresql": + return + op.execute("DROP TABLE IF EXISTS ui_event_hourly") diff --git a/backend/core/events.py b/backend/core/events.py new file mode 100644 index 0000000..6560908 --- /dev/null +++ b/backend/core/events.py @@ -0,0 +1,280 @@ +"""Product-event instrumentation: what people actually *do* in the UI. + +This is the durable, dimensioned tier that sits above ``core/metrics.py``'s +since-start counters. metrics.py answers "is traffic flowing right now"; this +module answers "did anyone ever get a location, and did search work" — over +weeks, across deploys. + +**Feature-flagged OFF.** Nothing here records anything unless +``THERMOGRAPH_EVENTS=1``. With the flag unset the beacon still answers 204 and +this module is inert, so shipping it is a no-op. + +Design constraints, in priority order: + +1. **No identifier, ever.** No cookie, no localStorage id, no fingerprint, no + user id — not even when the request carries a logged-in session. The unit of + record is an *hour bucket + a tuple of allowlisted enum values*, never a + person and never a row-per-interaction. That is a structural guarantee: there + is no column an identifier could be written into. +2. **No free text, no coordinates.** Every field is drawn from a fixed + allowlist (see ``SCHEMA``). An unrecognised name or value collapses to a + bounded "other" bucket rather than being stored. So the endpoint cannot be + turned into arbitrary storage, and cannot accidentally capture a search + query, a URL, or a lat/lon. +3. **No IP.** The caller's IP is used only as an in-memory rate-limit key for + the current minute (see ``metrics._rate_ok``) and is never passed in here. +4. **Never breaks a request.** Every entry point swallows its own errors. + +Storage: an hourly aggregate upsert into ``ui_event_hourly`` (a TimescaleDB +hypertable on Postgres — see ``alembic/versions/0003_ui_events.py``), plus one +JSONL line per event into the activity stream so Loki/Grafana can show the last +24h live. Neither carries anything person-linked. Off Postgres (dev, tests, +offline tooling) the aggregate sink degrades to a no-op and only the JSONL line +is written — the same dialect split ``data/store.py`` and ``core/metrics.py`` +already use. +""" +import datetime +import os +import threading + +from core import audit + +# --- feature flag ------------------------------------------------------------- +# Default OFF everywhere. Turn on per-environment (dev first, then beta, then +# prod) once the privacy copy in frontend_ssr/templates/privacy.html.j2 has been +# updated to match what is actually recorded. +ENABLED = os.environ.get("THERMOGRAPH_EVENTS", "").strip().lower() in ("1", "true", "yes", "on") + +DATABASE_URL = os.environ.get("THERMOGRAPH_DATABASE_URL", "").strip() +IS_POSTGRES = DATABASE_URL.startswith("postgresql") + + +def _libpq_dsn(url: str) -> str: + """A plain libpq URL psycopg accepts (mirrors climate_store/metrics).""" + return url.replace("+asyncpg", "").replace("+psycopg", "") + + +_PG_DSN = _libpq_dsn(DATABASE_URL) if IS_POSTGRES else "" + +# --- schema ------------------------------------------------------------------- +# Seven events. Every one answers a product question that cannot be answered from +# the request log alone; anything the request log *can* answer (page views, API +# usage, error rates) is deliberately NOT duplicated here. +# +# Three dimension slots are stored — ``view``, ``prop``, ``value`` — plus a +# referrer domain on ``view.open`` only (acquisition), so the key space stays +# small enough to eyeball. Each event declares which slots it uses and the exact +# allowlist for each. + +VIEWS = frozenset({ + "home", "calendar", "day", "compare", "score", "alerts", "legend", + "city", "cities", "records", "month", "glossary", "about", "privacy", +}) + +#: Where a location came from. The activation question: do visitors ever get +#: past "no place picked", and by which route? +PICK_METHODS = frozenset({"geolocate", "map", "search", "restore", "link"}) + +#: Did the location search actually work? ``hit`` = results shown, ``typo`` = +#: results only after respelling (places.suggest's ``corrected``), ``miss`` = +#: nothing found. The query string itself is never sent or stored. +SEARCH_OUTCOMES = frozenset({"hit", "typo", "miss"}) + +#: In-view controls worth knowing about. Anything not listed is dropped, so a +#: new control has to be added here deliberately (and reviewed for privacy). +CONTROLS = frozenset({ + "chart_metric", # value: the metric key + "unit", # value: F | C + "date", # value: today | past + "range", # value: the span bucket + "season", # value: season key or "custom" + "totals", # calendar totals mode + "dist_metric", # compare distribution metric + "comfort", # compare comfort slicer moved + "compare_add", # value: the location-count bucket after the add + "compare_remove", + "filter_sheet", # mobile filter sheet opened + "day_step", # day view prev/next +}) + +#: Bounded value vocabulary, shared across controls. A value outside this set is +#: stored as "other" — never verbatim. +VALUES = frozenset({ + # chart / distribution metrics + "tmax", "tmin", "feels", "humid", "wind", "gust", "precip", "dry", + # units + "F", "C", + # dates + "today", "past", "future", + # spans / counts (bucketed client-side, never a raw number) + "1", "2", "3", "4", "5plus", + "1y", "2y", "5y", "10y", "max", + # seasons + "winter", "spring", "summer", "fall", "custom", "all", + # generic + "on", "off", "other", +}) + +SHARE_KINDS = frozenset({"link", "png", "permalink"}) + +#: The alerts funnel. ``ok`` records whether the step succeeded, so a high +#: start/low create ratio is visible without any per-person tracking. +ALERT_STEPS = frozenset({"open", "start", "create", "push_on", "push_off", "test"}) + +#: Dead ends — the highest-signal events here. A spike in any of these is a bug +#: report nobody filed. +DEADENDS = frozenset({ + "grade_error", # /api/grade returned an error the user saw + "no_data", # a cell with no usable history + "geo_denied", # browser geolocation refused / timed out + "search_miss", # search returned nothing (paired with place.search=miss) + "not_found", # a 404 page was rendered + "offline", # the fetch failed with no network +}) + +OK = frozenset({"y", "n"}) + +#: name -> {slot: allowlist}. A slot absent from an event is always stored "". +SCHEMA: "dict[str, dict[str, frozenset]]" = { + # A view was opened (page load or SPA view). ``prop`` is how it was reached. + "view.open": {"view": VIEWS, "prop": frozenset({"direct", "nav", "link", "restore"})}, + # A location was chosen. NEVER carries coordinates — only the route taken. + "place.pick": {"view": VIEWS, "prop": PICK_METHODS}, + # A location search completed. Recorded server-side from api_suggest as well + # as client-side, so searches from every surface are covered. + "place.search": {"prop": SEARCH_OUTCOMES}, + # An in-view control was used. + "view.control": {"view": VIEWS, "prop": CONTROLS, "value": VALUES}, + # Share / export. + "share": {"view": VIEWS, "prop": SHARE_KINDS}, + # The alerts funnel. + "alert": {"prop": ALERT_STEPS, "value": OK}, + # A dead end the visitor actually hit. + "deadend": {"view": VIEWS, "prop": DEADENDS}, +} + +#: Anything unrecognised collapses here with every dimension blank — visible +#: enough to notice abuse, bounded enough to be harmless. +EVENT_OTHER = "other" +VALUE_OTHER = "other" + +SLOTS = ("view", "prop", "value") + +#: Max events per batched beacon payload. The client batches bursts (chart-metric +#: taps, slider drags) rather than firing a request each. +MAX_BATCH = 20 + +#: Max JSON body the beacon will read, in bytes. Must comfortably exceed +#: MAX_BATCH full-sized events (~90 bytes each) or the byte cap, not the batch +#: cap, becomes the binding limit and a legitimate full batch is silently +#: dropped. Still far too small for the endpoint to be worth abusing as storage. +MAX_BODY_BYTES = 4096 + + +def normalize(name: str, props: "dict | None" = None) -> "tuple[str, dict[str, str]]": + """An allowlisted ``(event, {view, prop, value})`` pair. + + The allowlist is the whole security model of an unauthenticated write + endpoint: it is what stops the beacon from becoming unbounded storage, a + spam sink, or an accidental PII channel. Unknown event -> ``EVENT_OTHER`` + with blank dimensions; unknown value in a known slot -> ``VALUE_OTHER``; + a slot the event does not declare -> dropped entirely. + """ + name = (name or "").strip() + spec = SCHEMA.get(name) + if spec is None: + return EVENT_OTHER, {s: "" for s in SLOTS} + props = props if isinstance(props, dict) else {} + out: "dict[str, str]" = {} + for slot in SLOTS: + allowed = spec.get(slot) + if allowed is None: + out[slot] = "" + continue + raw = props.get(slot) + raw = raw.strip() if isinstance(raw, str) else "" + out[slot] = raw if raw in allowed else (VALUE_OTHER if raw else "") + return name, out + + +# --- durable aggregate sink --------------------------------------------------- +# One row per (hour, event, view, prop, value, referrer). No row is ever written +# per interaction, so the table cannot hold a sequence of one person's actions +# even in principle. + +_local = threading.local() + + +def _conn(): + """Thread-local autocommit psycopg connection, or None when Postgres is off + or unreachable. Mirrors data/climate_store.py.""" + if not IS_POSTGRES: + return None + conn = getattr(_local, "conn", None) + if conn is not None and not conn.closed: + return conn + try: + import psycopg # local import: only the Postgres path needs it + _local.conn = psycopg.connect(_PG_DSN, autocommit=True) + except Exception: # noqa: BLE001 - degrade to "no durable sink", never raise + _local.conn = None + return _local.conn + + +def _hour_bucket() -> datetime.datetime: + now = datetime.datetime.now(datetime.timezone.utc) + return now.replace(minute=0, second=0, microsecond=0) + + +_UPSERT = """ + INSERT INTO ui_event_hourly (bucket, event, view, prop, value, referrer, count) + VALUES (%s, %s, %s, %s, %s, %s, 1) + ON CONFLICT (bucket, event, view, prop, value, referrer) + DO UPDATE SET count = ui_event_hourly.count + 1 +""" + + +def _upsert(event: str, dims: "dict[str, str]", referrer: str) -> None: + conn = _conn() + if conn is None: + return + try: + conn.execute(_UPSERT, (_hour_bucket(), event, dims["view"], dims["prop"], + dims["value"], referrer)) + except Exception: # noqa: BLE001 - a broken connection retires itself below + try: + conn.close() + except Exception: # noqa: BLE001 + pass + _local.conn = None + + +# --- public entry point ------------------------------------------------------- + +def record(name: str, props: "dict | None" = None, + referer: "str | None" = None, host: "str | None" = None) -> None: + """Record one product event. Best-effort, silent, and a no-op unless the + feature flag is on. + + ``referer``/``host`` come from the request's own headers (a client-supplied + referrer would be forgeable and buys nothing) and are reduced to a bare + domain by ``metrics.normalize_referrer`` — never a full URL, which can carry + a search query or a private path. The referrer dimension is kept only for + ``view.open``; every other event stores "" there, which is what keeps the + key space small. + """ + if not ENABLED: + return + try: + # Import here, not at module scope: metrics imports cleanly on its own and + # this keeps the dependency one-directional at import time. + from core import metrics + event, dims = normalize(name, props) + referrer = metrics.normalize_referrer(referer, host) if event == "view.open" else "" + _upsert(event, dims, referrer) + # Second sink: one structured line per event into the activity stream, so + # the Loki/Grafana stack can show the last 24h live without waiting on an + # hourly rollup. Carries the same allowlisted enums and nothing else. + audit.log_activity("ui.event", {"event": event, **dims, "referrer": referrer}) + except Exception: # noqa: BLE001 - instrumentation must never break a request + pass diff --git a/backend/core/metrics.py b/backend/core/metrics.py index 36b085b..e306d3e 100644 --- a/backend/core/metrics.py +++ b/backend/core/metrics.py @@ -705,12 +705,38 @@ def record_outbound(phase: str, outcome: str) -> None: # Per-IP token bucket for the unauthenticated event beacon. Bounded: the map is # cleared whenever the minute rolls, so it holds at most one minute of clients. -_EVENT_RATE_PER_MIN = 30 +# +# 30/min was sized for the four coarse home.* counters (at most a handful per +# visit). Interaction-level events (chart-metric taps, a comfort slider drag on +# the compare page) are burstier by an order of magnitude, and a batched beacon +# spends one token per event in the batch — at 30 the limiter silently truncates +# ordinary use, which is worse than useless because the loss is invisible and +# biased toward the most engaged visitors. 120/min is still a hard ceiling on +# what one IP can write per minute. Env-tunable so it can be tightened on the +# box without a deploy if the endpoint is ever abused. +_EVENT_RATE_PER_MIN = int(os.environ.get("THERMOGRAPH_EVENT_RATE_PER_MIN", "120") or 120) _rate_lock = threading.Lock() _rate_minute = -1 _rate_counts: "dict[str, int]" = {} +def reset_rate_limiter() -> None: + """Drop every token bucket. For tests — one test's beacon traffic must not + rate-limit the next one's, since TestClient always presents the same IP.""" + global _rate_minute + with _rate_lock: + _rate_minute = -1 + _rate_counts.clear() + + +def rate_ok(ip: "str | None") -> bool: + """Public form of the beacon's per-IP token bucket, so a caller that fans one + event out to several sinks (see web/app.py's ``_record_event``) spends exactly + one token instead of one per sink. The IP is only ever a key in a map that is + cleared every minute — it is never persisted anywhere.""" + return _rate_ok(ip) + + def _rate_ok(ip: "str | None") -> bool: global _rate_minute key = ip or "?" @@ -725,13 +751,17 @@ def _rate_ok(ip: "str | None") -> bool: def record_event(name: str, referer: "str | None" = None, - host: "str | None" = None, ip: "str | None" = None) -> None: + host: "str | None" = None, ip: "str | None" = None, + rate_limit: bool = True) -> None: """Count one product event. The name is allowlisted and the referrer is taken from the request's own Referer header (never client-supplied, which would be trivially forgeable). Best-effort and silent: the caller always answers 204, - so a rejected or rate-limited event tells a prober nothing.""" + so a rejected or rate-limited event tells a prober nothing. + + ``rate_limit=False`` skips the per-IP bucket for a caller that already spent + the token through ``rate_ok`` — see web/app.py's ``_record_event``.""" try: - if not _rate_ok(ip): + if rate_limit and not _rate_ok(ip): return event = normalize_event(name) # An unrecognized event carries no referrer dimension — one bucket total. diff --git a/backend/tests/core/test_events.py b/backend/tests/core/test_events.py new file mode 100644 index 0000000..9af9212 --- /dev/null +++ b/backend/tests/core/test_events.py @@ -0,0 +1,119 @@ +"""Unit tests for the UI product-event schema (core/events.py). + +The allowlist in ``events.SCHEMA`` is the entire security and privacy model of an +unauthenticated write endpoint, so most of what is worth asserting here is what +does NOT get through: free text, coordinates, unknown names, unknown values. +""" +import importlib + +from core import events + + +def _fresh(monkeypatch, enabled=True): + m = importlib.reload(events) + monkeypatch.setattr(m, "ENABLED", enabled) + return m + + +def test_known_event_keeps_only_declared_slots(monkeypatch): + m = _fresh(monkeypatch) + name, dims = m.normalize("view.open", {"view": "calendar", "prop": "nav", "value": "tmax"}) + assert name == "view.open" + # view.open declares view + prop but not value, so value is dropped entirely + # rather than stored — an undeclared slot can never carry anything. + assert dims == {"view": "calendar", "prop": "nav", "value": ""} + + +def test_unknown_event_collapses_with_blank_dimensions(monkeypatch): + m = _fresh(monkeypatch) + name, dims = m.normalize("evil.event", {"view": "calendar", "prop": "nav"}) + assert name == m.EVENT_OTHER + assert dims == {"view": "", "prop": "", "value": ""} + + +def test_unknown_value_in_a_known_slot_is_bucketed_never_stored(monkeypatch): + """The property that makes the endpoint safe: nothing a client sends is ever + persisted verbatim.""" + m = _fresh(monkeypatch) + _, dims = m.normalize("place.pick", {"view": "home", "prop": "47.6,-122.3"}) + assert dims["prop"] == m.VALUE_OTHER + _, dims = m.normalize("view.control", {"view": "home", "prop": "chart_metric", + "value": "seattle wa"}) + assert dims["value"] == m.VALUE_OTHER + + +def test_non_string_props_do_not_crash_or_leak(monkeypatch): + m = _fresh(monkeypatch) + for props in (None, [], "x", {"view": 47.6}, {"view": {"lat": 1}}): + name, dims = m.normalize("place.pick", props) + assert name == "place.pick" + assert set(dims) == {"view", "prop", "value"} + assert all(isinstance(v, str) for v in dims.values()) + + +def test_schema_slots_are_all_recognised(monkeypatch): + """Guard against a typo'd slot name silently making an event dimensionless.""" + m = _fresh(monkeypatch) + for name, spec in m.SCHEMA.items(): + assert spec, name + assert set(spec) <= set(m.SLOTS), name + for slot, allowed in spec.items(): + assert isinstance(allowed, frozenset) and allowed, (name, slot) + + +def test_no_slot_can_hold_free_text_by_construction(monkeypatch): + """Every allowlisted value is a short, boring token. If this ever fails, + something person-identifying has been added to the vocabulary.""" + m = _fresh(monkeypatch) + for spec in m.SCHEMA.values(): + for allowed in spec.values(): + for v in allowed: + assert v.replace("_", "").replace("-", "").isalnum(), v + assert len(v) <= 20, v + + +def test_record_is_inert_when_the_flag_is_off(monkeypatch): + m = _fresh(monkeypatch, enabled=False) + calls = [] + monkeypatch.setattr(m, "_upsert", lambda *a: calls.append(a)) + monkeypatch.setattr(m.audit, "log_activity", lambda *a, **k: calls.append(a)) + m.record("view.open", {"view": "home"}) + assert calls == [] + + +def test_record_writes_one_activity_line_with_only_allowlisted_fields(monkeypatch): + m = _fresh(monkeypatch) + lines = [] + monkeypatch.setattr(m, "_upsert", lambda *a: None) + monkeypatch.setattr(m.audit, "log_activity", lambda tag, rec: lines.append((tag, rec))) + m.record("view.open", {"view": "city", "prop": "nav"}, + referer="https://news.ycombinator.com/item?id=1", host="thermograph.org") + (tag, rec), = lines + assert tag == "ui.event" + # The referrer is reduced to a bare domain — never the URL, which can carry a + # search query or a private path. + assert rec == {"event": "view.open", "view": "city", "prop": "nav", + "value": "", "referrer": "news.ycombinator.com"} + + +def test_referrer_is_kept_only_for_view_open(monkeypatch): + """Acquisition needs it; nothing else does, and dropping it everywhere else is + what keeps the key space small enough to eyeball.""" + m = _fresh(monkeypatch) + lines = [] + monkeypatch.setattr(m, "_upsert", lambda *a: None) + monkeypatch.setattr(m.audit, "log_activity", lambda tag, rec: lines.append(rec)) + m.record("share", {"view": "home", "prop": "link"}, + referer="https://reddit.com/r/weather", host="thermograph.org") + assert lines[0]["referrer"] == "" + + +def test_record_never_raises(monkeypatch): + m = _fresh(monkeypatch) + + def boom(*a, **k): + raise RuntimeError("sink down") + + monkeypatch.setattr(m, "_upsert", boom) + monkeypatch.setattr(m.audit, "log_activity", boom) + m.record("view.open", {"view": "home"}) # must not propagate diff --git a/backend/tests/web/test_event_beacon.py b/backend/tests/web/test_event_beacon.py new file mode 100644 index 0000000..cff58f3 --- /dev/null +++ b/backend/tests/web/test_event_beacon.py @@ -0,0 +1,141 @@ +"""Route-level tests for the public product-event beacon (POST /api/v2/event). + +The endpoint is unauthenticated and reachable by anything on the internet — on a +site where ~60% of inbound traffic is crawlers and scanners — so these tests are +mostly about what it REFUSES to do: grow storage, echo anything back, or accept +a payload big enough to be worth abusing. +""" +import json + +import pytest +from fastapi.testclient import TestClient + +from core import events +from web import app as appmod + +EVENT_URL = "/thermograph/api/v2/event" +# Browsers set this on fetch/sendBeacon; a bare scanner POST does not. +FIRST_PARTY = {"sec-fetch-site": "same-origin"} + + +@pytest.fixture(autouse=True) +def _fresh_rate_limiter(): + """TestClient always presents the same client IP, so without this one test's + beacon traffic silently rate-limits the next one's.""" + from core import metrics + metrics.reset_rate_limiter() + yield + metrics.reset_rate_limiter() + + +@pytest.fixture +def client(): + return TestClient(appmod.app) + + +@pytest.fixture +def recorded(monkeypatch): + """Capture what reaches the recorder, with the flag forced on.""" + seen = [] + monkeypatch.setattr(events, "ENABLED", True) + monkeypatch.setattr(events, "_upsert", lambda *a: None) + monkeypatch.setattr(events.audit, "log_activity", lambda tag, rec: seen.append(rec)) + return seen + + +def test_beacon_always_answers_204_and_returns_no_body(client, recorded): + """Counted, rejected, or rate-limited must be indistinguishable to a prober.""" + for body in ({"event": "view.open", "props": {"view": "home"}}, + {"event": "not-a-real-event"}, + {"nonsense": True}, + [1, 2, 3], + "plain string"): + r = client.post(EVENT_URL, content=json.dumps(body), headers=FIRST_PARTY) + assert r.status_code == 204 + assert r.content == b"" + + +def test_valid_event_is_recorded_with_allowlisted_dimensions(client, recorded): + client.post(EVENT_URL, json={"event": "view.control", + "props": {"view": "compare", "prop": "unit", "value": "C"}}, + headers=FIRST_PARTY) + assert recorded == [{"event": "view.control", "view": "compare", "prop": "unit", + "value": "C", "referrer": ""}] + + +def test_batched_payload_is_accepted_and_capped(client, recorded): + items = [{"event": "share", "props": {"prop": "link"}}] * (events.MAX_BATCH + 5) + client.post(EVENT_URL, json={"events": items}, headers=FIRST_PARTY) + assert len(recorded) == events.MAX_BATCH + + +def test_a_full_batch_of_real_events_fits_under_the_byte_cap(client, recorded): + """Regression: if MAX_BODY_BYTES ever drops below a full MAX_BATCH payload, + the byte cap silently eats legitimate batches instead of abusive ones.""" + items = [{"event": "view.control", + "props": {"view": "compare", "prop": "chart_metric", "value": "precip"}} + ] * events.MAX_BATCH + assert len(json.dumps({"events": items})) < events.MAX_BODY_BYTES + client.post(EVENT_URL, json={"events": items}, headers=FIRST_PARTY) + assert len(recorded) == events.MAX_BATCH + + +def test_oversized_body_is_dropped_without_being_parsed(client, recorded): + """The endpoint must not become storage: a body past the cap is never read + into a record at all.""" + fat = {"event": "view.open", "props": {"view": "home"}, + "junk": "x" * (events.MAX_BODY_BYTES * 4)} + r = client.post(EVENT_URL, content=json.dumps(fat), headers=FIRST_PARTY) + assert r.status_code == 204 + assert recorded == [] + + +def test_free_text_and_coordinates_never_survive(client, recorded): + """Anything a client puts in a dimension slot is allowlisted or bucketed — + a leaked query string or lat/lon cannot reach a sink.""" + client.post(EVENT_URL, json={"event": "place.pick", + "props": {"view": "home", "prop": "47.6062,-122.3321"}}, + headers=FIRST_PARTY) + client.post(EVENT_URL, json={"event": "view.control", + "props": {"view": "home", "prop": "chart_metric", + "value": "user@example.com"}}, + headers=FIRST_PARTY) + stored = {v for rec in recorded for v in rec.values()} + assert not any("47.6" in s or "@" in s for s in stored) + + +def test_cross_site_post_is_dropped(client, recorded): + r = client.post(EVENT_URL, json={"event": "view.open", "props": {"view": "home"}}, + headers={"sec-fetch-site": "cross-site"}) + assert r.status_code == 204 + assert recorded == [] + + +def test_flag_off_records_nothing(client, monkeypatch): + """The whole feature ships inert: with THERMOGRAPH_EVENTS unset the beacon + still answers, and nothing reaches the durable sink.""" + seen = [] + monkeypatch.setattr(events, "ENABLED", False) + monkeypatch.setattr(events, "_upsert", lambda *a: seen.append(a)) + monkeypatch.setattr(events.audit, "log_activity", lambda tag, rec: seen.append(rec)) + r = client.post(EVENT_URL, json={"event": "view.open", "props": {"view": "home"}}, + headers=FIRST_PARTY) + assert r.status_code == 204 + assert seen == [] + + +def test_legacy_single_event_shape_still_counts(client, monkeypatch): + """The four home.* aggregate counters already in production keep working — + they are unflagged and predate the schema.""" + from core import metrics + seen = [] + monkeypatch.setattr(metrics, "_store", type("S", (), { + "record_event": lambda self, *a: seen.append(a)})()) + client.post(EVENT_URL, json={"event": "home.locate"}, headers=FIRST_PARTY) + assert seen and seen[0][0] == "home.locate" + + +def test_beacon_is_not_counted_as_inbound_traffic(client): + """It reports traffic; counting it would double every interaction.""" + from core import metrics + assert metrics.classify_inbound(EVENT_URL, "/thermograph") == "event" diff --git a/backend/web/app.py b/backend/web/app.py index 6953b73..048a738 100644 --- a/backend/web/app.py +++ b/backend/web/app.py @@ -20,6 +20,7 @@ from fastapi.responses import JSONResponse, RedirectResponse from accounts import api_accounts from api import content_routes from core import audit +from core import events from data import climate from notifications import digest from notifications import discord_bot @@ -452,6 +453,16 @@ def api_suggest(q: str = Query(..., min_length=1)): results, corrected = places.suggest(q, _no_upstream, _SUGGEST_LIMIT) except Exception as e: # noqa: BLE001 - nothing at all to serve raise HTTPException(status_code=502, detail=f"geocoding failed: {e}") + # "Did search work?" is recorded HERE, not in the browser: the server already + # knows the answer, every surface that searches is covered for free, and the + # query string never leaves this handler — only the three-way outcome (found / + # found-after-respelling / nothing) is stored. Inert unless THERMOGRAPH_EVENTS + # is on. NOTE: the type-ahead fires once per debounced keystroke, so this + # counts *keystroke batches*, not distinct searches — read the miss RATE, not + # the miss count. + events.record("place.search", + {"prop": "hit" if results and not corrected + else "typo" if results else "miss"}) return {"results": results, "corrected": corrected} @@ -779,36 +790,110 @@ def api_metrics(request: Request): return snap -async def api_event(request: Request) -> Response: - """Record one product event (a tap on "use my location", a digest signup, …). +def _same_origin(request: Request) -> bool: + """A soft first-party check for the public beacon. - Deliberately minimal: the body carries only an allowlisted event name. The - referrer is read from this request's own Referer header — a client-supplied - one would be forgeable and buys nothing — and is reduced to a bare domain. - No cookies, no per-visitor id, no full URLs. - - Always answers 204, whether the event was counted, rejected by the allowlist, - or dropped by the rate limiter, so probing the endpoint reveals nothing. That - also suits navigator.sendBeacon, which ignores the response. + Every browser that can run our JS sets ``Sec-Fetch-Site`` on a fetch or + sendBeacon; a curl/scanner POST does not, and a cross-site forgery reports + ``cross-site``. Neither header is a security boundary (both are trivially + spoofable by anything that bothers), but together they filter out the naive + majority — which is exactly what "don't let the analytics table fill with + junk" needs. Anything suspicious is dropped silently; the response is 204 + either way. """ - name = "" + site = request.headers.get("sec-fetch-site") + if site is not None: + return site in ("same-origin", "same-site", "none") + origin = request.headers.get("origin") + if origin: + host = (request.headers.get("host") or "").partition(":")[0].lower() + return host != "" and origin.rpartition("/")[2].partition(":")[0].lower().removeprefix("www.") \ + == host.removeprefix("www.") + # No Sec-Fetch-Site and no Origin: an old browser, or a bot. Keep it — the + # allowlist + rate limiter already bound the damage — but it is the bucket + # to look at first if the numbers ever look wrong. + return True + + +async def _read_capped(request: Request, limit: int) -> bytes: + """At most ``limit`` bytes of the request body. A real beacon is ~60 bytes, + so anything larger is either a bug or an attempt to use the endpoint as + storage: stop reading rather than buffering it.""" + declared = request.headers.get("content-length") + if declared and declared.isdigit() and int(declared) > limit: + return b"" + chunks: "list[bytes]" = [] + size = 0 + async for chunk in request.stream(): + size += len(chunk) + if size > limit: + return b"" + chunks.append(chunk) + return b"".join(chunks) + + +async def api_event(request: Request) -> Response: + """Record one or more product events (a tap on "use my location", a chart + metric switch, a dead end the visitor hit, …). + + Deliberately minimal: the body carries an allowlisted event name and a few + allowlisted enum properties — no free text, no coordinates, no identifier of + any kind, and the session cookie is deliberately never read. The referrer is + taken from this request's own Referer header (a client-supplied one would be + forgeable and buys nothing) and reduced to a bare domain. + + Body shapes accepted:: + + {"event": "view.open", "props": {"view": "calendar", "prop": "nav"}} + {"events": [{"event": ..., "props": {...}}, ...]} # batched, <= MAX_BATCH + + Always answers 204 — counted, rejected by the allowlist, over the body cap, + or dropped by the rate limiter all look identical, so probing the endpoint + reveals nothing. That also suits navigator.sendBeacon, which ignores the + response entirely. + """ + raw = await _read_capped(request, events.MAX_BODY_BYTES) + if not raw or not _same_origin(request): + return Response(status_code=204) try: - body = await request.json() - if isinstance(body, dict): - name = str(body.get("event") or "") - except Exception: # noqa: BLE001 - a junk body is just a no-op event - pass - if name: - await run_in_threadpool( - metrics.record_event, - name, - referer=request.headers.get("referer"), - host=request.headers.get("host"), - ip=_client_ip(request), - ) + body = json.loads(raw) + except Exception: # noqa: BLE001 - a junk body is just a no-op + return Response(status_code=204) + if not isinstance(body, dict): + return Response(status_code=204) + + batch = body.get("events") + if not isinstance(batch, list): + batch = [body] + batch = [e for e in batch[:events.MAX_BATCH] if isinstance(e, dict)] + + referer = request.headers.get("referer") + host = request.headers.get("host") + # The IP is a rate-limit key for the current minute and nothing else: it is + # never written to the counters, the aggregate table, or the activity log. + ip = _client_ip(request) + for item in batch: + name = str(item.get("event") or "") + if not name: + continue + props = item.get("props") + await run_in_threadpool(_record_event, name, props, referer, host, ip) return Response(status_code=204) +def _record_event(name, props, referer, host, ip) -> None: + """Fan one event out to both tiers (blocking; called via the threadpool). + + ``metrics`` keeps the since-start live counters the ops endpoint already + serves — untouched, and still the rate-limit gate. ``events`` adds the + durable, dimensioned hourly aggregate, and is inert unless THERMOGRAPH_EVENTS + is on.""" + if not metrics.rate_ok(ip): + return + metrics.record_event(name, referer=referer, host=host, rate_limit=False) + events.record(name, props, referer=referer, host=host) + + # --- API versioning -------------------------------------------------------- # Non-backward-compatible additions land in a new version; every version stays # mounted and served simultaneously. v1 is the original contract (grade/geocode); diff --git a/frontend/app.py b/frontend/app.py index 4eb4d61..ee5ebac 100644 --- a/frontend/app.py +++ b/frontend/app.py @@ -96,6 +96,12 @@ def _page(name): verify = content.head_verify_html() if verify: html = html.replace("", f"\n {verify}", 1) + # Same flag stamp the SSR base template applies: track.js sends + # nothing unless it finds it. Folded into the memoized template + # because it is constant for the process's life. + if content.EVENTS_ENABLED: + html = html.replace('', + '', 1) _template.append(html) return _template[0] diff --git a/frontend/content.py b/frontend/content.py index 6981b25..5d75444 100644 --- a/frontend/content.py +++ b/frontend/content.py @@ -36,6 +36,14 @@ BASE = f"/{_BASE}" if _BASE else "" # references stay relative (ASSET_BASE == BASE). Repo-split Stage 5. ASSET_BASE = os.environ.get("THERMOGRAPH_API_BASE_PUBLIC", "").rstrip("/") or BASE +# UI product-event instrumentation (backend/core/events.py). OFF unless this is +# set, and set per-environment -- the flag lives in BOTH services because each +# renders HTML: this one stamps data-tg-events on , which is the only +# thing that makes track.js send anything, and backend's copy is what decides +# whether the beacon records what arrives. Both must be on for events to flow. +EVENTS_ENABLED = os.environ.get("THERMOGRAPH_EVENTS", "").strip().lower() in ( + "1", "true", "yes", "on") + _env = Environment( loader=FileSystemLoader(paths.TEMPLATES_DIR), autoescape=select_autoescape(["html", "xml", "j2"]), @@ -76,6 +84,7 @@ def _respond_html(request: Request, template: str, **ctx) -> Response: # origin recovers exactly today's behavior in the default topology. asset_base_url = ASSET_BASE if ASSET_BASE.startswith(("http://", "https://")) else f"{o}{ASSET_BASE}" ctx.setdefault("unit_default", fmt.current_unit()) + ctx.setdefault("events_enabled", EVENTS_ENABLED) html = _env.get_template(template).render( base=BASE, asset_base=ASSET_BASE, asset_base_url=asset_base_url, origin=o, base_url=f"{o}{BASE}", **ctx) diff --git a/frontend/static/app.js b/frontend/static/app.js index 47fa6be..caf1a31 100644 --- a/frontend/static/app.js +++ b/frontend/static/app.js @@ -6,7 +6,7 @@ import { loadView, TTL, prefetchViews } from "./cache.js"; import { initFindButton, setFindLabel } from "./mappicker.js"; import { W, PW, H, PL, PR, plotTop, plotBot, xLabY, setChartWidth, chartPalette, tempChart, precipChart, dryChart, attachChartHover } from "./chart.js"; -import { track } from "./digest.js"; +import { track, trackLegacy, currentView } from "./track.js"; import { TIER_COLORS, SCALE_TEMP, drynessColor, pctOrd, esc, todayISO, placeLabel, tierKeySegs, GUIDE_LINK, fmtPrecip, fmtWind, fmtHumid } from "./shared.js"; @@ -46,7 +46,10 @@ function selectLocation(lat, lon) { const findBtn = document.getElementById("find-btn"); const locLabel = document.getElementById("loc-label"); initFindButton(findBtn, "Find a location", - () => selected, (lat, lon) => selectLocation(lat, lon)); + () => selected, (lat, lon, method) => { + track("place.pick", { view: "home", prop: method || "map" }); + selectLocation(lat, lon); + }); // ---- hero ---- // The hero's grade card is server-rendered showing the most unusual city we're @@ -78,7 +81,7 @@ if (heroLocate && !canLocate) { } heroLocate?.addEventListener("click", () => { - track("home.locate"); + trackLegacy("home.locate"); const label = heroLocate.textContent; const restore = () => { heroLocate.disabled = false; @@ -93,10 +96,16 @@ heroLocate?.addEventListener("click", () => { navigator.geolocation.getCurrentPosition( (pos) => { restore(); + // Records only THAT a location was obtained and by which route — never + // the coordinates, which are location data about a person. + track("place.pick", { view: "home", prop: "geolocate" }); selectLocation(pos.coords.latitude, pos.coords.longitude); }, (err) => { restore(); + // A dead end the visitor actually hit: the button looked like it worked + // and then nothing happened. Worth a counter of its own. + track("deadend", { view: "home", prop: "geo_denied" }); // Always say something: a declined prompt or a timeout must not look // like the button did nothing. locateMsg( @@ -160,7 +169,10 @@ async function runGrade() { url, ttl: TTL.grade, seq: ++gradeSeq, current: () => gradeSeq, spinner: () => { results.innerHTML = ``; }, onData: render, - onError: (err) => { results.innerHTML = `

${err.message}

`; }, + onError: (err) => { + results.innerHTML = `

${err.message}

`; + track("deadend", { view: "home", prop: navigator.onLine === false ? "offline" : "grade_error" }); + }, }); } @@ -505,6 +517,7 @@ function buildChart(data) { const b = e.target.closest("button[data-metric]"); if (!b || b.dataset.metric === chartMetric) return; chartMetric = b.dataset.metric; + track("view.control", { view: "home", prop: "chart_metric", value: chartMetric }); try { localStorage.setItem("thermograph:chartMetric", chartMetric); } catch (e) {} buildChart(data); }); @@ -514,6 +527,7 @@ function buildChart(data) { // ---- share ---- function copyShareLink() { if (!selected) return; + track("share", { view: "home", prop: "link" }); const url = `${location.origin}${location.pathname}${locHash(selected.lat, selected.lon, dateInput.value)}`; navigator.clipboard.writeText(url).then( () => flashBtn("btn-link", "✓ Copied!"), @@ -590,6 +604,7 @@ function downloadChartPng() { a.click(); setTimeout(() => URL.revokeObjectURL(a.href), 1000); flashBtn("btn-png", "✓ Saved"); + track("share", { view: "home", prop: "png" }); }, "image/png"); }; img.src = url; @@ -619,3 +634,11 @@ function restoreFromHash() { selectLocation(loc.lat, loc.lon); } restoreFromHash(); + +// One view.open per page load. `prop` is how the view was reached: an explicit +// link (a shared permalink carries a location hash), a remembered place, or a +// cold arrival. This is the denominator every other event is read against. +track("view.open", { + view: currentView(), + prop: location.hash ? "link" : (selected ? "restore" : "direct"), +}); diff --git a/frontend/static/calendar.html b/frontend/static/calendar.html index 96fc59a..0f306fd 100644 --- a/frontend/static/calendar.html +++ b/frontend/static/calendar.html @@ -121,6 +121,9 @@ + + diff --git a/frontend/static/compare.html b/frontend/static/compare.html index 17e2e2c..153cd3b 100644 --- a/frontend/static/compare.html +++ b/frontend/static/compare.html @@ -177,6 +177,9 @@ + + diff --git a/frontend/static/day.html b/frontend/static/day.html index 71b0c1d..01219d2 100644 --- a/frontend/static/day.html +++ b/frontend/static/day.html @@ -83,6 +83,9 @@ + + diff --git a/frontend/static/digest.js b/frontend/static/digest.js index cfef64e..745ee3f 100644 --- a/frontend/static/digest.js +++ b/frontend/static/digest.js @@ -1,40 +1,14 @@ -// Digest signup + product-event beacons. Loaded on every page (base.html.j2). +// Digest signup form. Loaded on every page (base.html.j2). // -// Both halves degrade: the form is a real
that works with -// this file blocked, and the beacons are fire-and-forget with no UI depending -// on them. +// It degrades: the form is a real that works with this +// file blocked. +// +// The product-event beacon used to live here; it now lives in track.js, which +// also owns the [data-event] click delegation this file used to do. `track` is +// re-exported for the pages that import it from here. -// Backend's own origin+base (+ pinned API version), shared with account.js -// rather than a second independent import.meta.url-derived copy (repo-split -// Stage 5 — this file, like account.js, is always served by backend, possibly -// from a page on a different origin once frontend/backend genuinely split). -import { uv } from "./account.js"; - -/** Report one product event. Aggregate-only: the server takes the referrer from - * the request itself, so nothing identifying is sent from here. */ -export function track(event) { - try { - const url = uv("event"); - const body = JSON.stringify({ event }); - // sendBeacon survives the page unloading, which a fetch() started on a link - // click usually does not — that's the whole point for nav/records clicks. - // Spec limitation: sendBeacon has no credentials/header control at all, so - // it can never carry the auth cookie cross-origin — fine here, event - // tracking is anonymous and needs no auth either way. - if (navigator.sendBeacon) { - navigator.sendBeacon(url, new Blob([body], { type: "application/json" })); - } else { - fetch(url, { method: "POST", body, headers: { "Content-Type": "application/json" }, - keepalive: true }).catch(() => {}); - } - } catch { /* instrumentation must never break the page */ } -} - -// Anything carrying data-event reports itself when activated. -document.addEventListener("click", (e) => { - const el = e.target.closest?.("[data-event]"); - if (el) track(el.dataset.event); -}); +export { track, trackLegacy } from "./track.js"; +import { trackLegacy } from "./track.js"; // --- digest form ------------------------------------------------------------- for (const form of document.querySelectorAll("form[data-digest]")) { @@ -63,7 +37,7 @@ for (const form of document.querySelectorAll("form[data-digest]")) { // left to read; the form is done either way. form.querySelector(".digest-row")?.setAttribute("hidden", ""); form.querySelector(".digest-note")?.setAttribute("hidden", ""); - track("home.digest_signup"); + trackLegacy("home.digest_signup"); } } catch { if (status) { diff --git a/frontend/static/legend.html b/frontend/static/legend.html index cd33c7e..15b7492 100644 --- a/frontend/static/legend.html +++ b/frontend/static/legend.html @@ -115,6 +115,9 @@ document.getElementById("temp-scale").innerHTML = SCALE_TEMP.map(row).join(""); document.getElementById("rain-scale").innerHTML = SCALE_RAIN.map(row).join(""); + + diff --git a/frontend/static/mappicker.js b/frontend/static/mappicker.js index 068b3ba..2a9e783 100644 --- a/frontend/static/mappicker.js +++ b/frontend/static/mappicker.js @@ -48,7 +48,7 @@ function build() { overlay.querySelector(".mp-close").onclick = close; // Tapping the dimmed backdrop (outside the modal) closes the picker. overlay.addEventListener("pointerdown", (e) => { if (e.target === overlay) close(); }); - confirmBtn.onclick = () => finish(pin); + confirmBtn.onclick = () => finish(pin, "map"); // Live suggestions (top 5, single-letter-typo tolerant) as the user types: // debounced so a fast typist doesn't fire a request per keystroke, aborted @@ -131,7 +131,7 @@ function renderSug(list) { sugEl.hidden = true; searchInput.value = r.name; setPin(r.lat, r.lon, 10); - finish({ lat: r.lat, lon: r.lon }); + finish({ lat: r.lat, lon: r.lon }, "search"); }; // Keep one highlight: hovering moves it off whatever the arrows chose. li.addEventListener("pointerenter", () => @@ -152,11 +152,14 @@ function setPin(lat, lon, zoom) { hintEl.textContent = "Pin set. Confirm below, or tap elsewhere to move it."; } -function finish(p) { +// `method` tells the caller HOW the spot was chosen ("search" = picked from +// the suggestion list, "map" = pin dropped/confirmed on the map), so a page can +// report that dimension without the picker knowing anything about analytics. +function finish(p, method) { if (!p) return; const cb = onPickCb; close(); - if (cb) cb(p.lat, p.lon); + if (cb) cb(p.lat, p.lon, method); } export function open(cur, onPick) { diff --git a/frontend/static/score.html b/frontend/static/score.html index 69af6e5..2235393 100644 --- a/frontend/static/score.html +++ b/frontend/static/score.html @@ -70,6 +70,9 @@ + + diff --git a/frontend/static/subscriptions.html b/frontend/static/subscriptions.html index fa88637..1ed0e6b 100644 --- a/frontend/static/subscriptions.html +++ b/frontend/static/subscriptions.html @@ -63,6 +63,9 @@ + + diff --git a/frontend/static/track.js b/frontend/static/track.js new file mode 100644 index 0000000..5ce4bf4 --- /dev/null +++ b/frontend/static/track.js @@ -0,0 +1,129 @@ +// Product-event beacon: what people actually do in the UI. +// +// Off by default. The server sets `data-tg-events="1"` on only when +// THERMOGRAPH_EVENTS is on for that environment, so with the flag unset this +// file costs one dataset read per page and sends nothing at all. +// +// What it will NOT send, by construction: +// * no identifier of any kind — no cookie, no localStorage id, no fingerprint, +// no user id even when signed in (sendBeacon cannot attach credentials +// cross-origin anyway, and the server never reads the session on this route); +// * no free text — never a search query, never a place name; +// * no coordinates — `place.pick` records HOW a location was chosen, not where; +// * no URLs — the server derives a bare referrer domain from its own headers. +// Every field is an enum from a fixed allowlist mirrored server-side in +// backend/core/events.py; anything else is dropped there. +// +// Global Privacy Control / Do Not Track are honoured: a visitor who has signalled +// either sends nothing. That biases the numbers slightly downward and is worth it. + +// Backend's own origin + base + pinned API version, shared with account.js +// rather than a second independent copy. +import { uv } from "./account.js"; + +const URL_ = uv("event"); + +const optedOut = + navigator.globalPrivacyControl === true || + navigator.doNotTrack === "1" || + window.doNotTrack === "1"; + +const enabled = () => + document.documentElement.dataset.tgEvents === "1" && !optedOut; + +// Batch rather than one request per interaction: chart-metric taps and slider +// drags arrive in bursts, and ~0.03 req/s of real traffic does not need a +// request each. Flushed on a short timer and unconditionally when the page is +// hidden or unloaded, which is the case a plain fetch() would lose. +const MAX_BATCH = 20; // mirrors events.MAX_BATCH server-side +const FLUSH_MS = 4000; +let queue = []; +let timer = 0; + +function send(items) { + try { + const body = JSON.stringify({ events: items }); + if (navigator.sendBeacon) { + // sendBeacon survives the page unloading, which a fetch() started on a + // link click usually does not — that's the whole point for nav clicks. + navigator.sendBeacon(URL_, new Blob([body], { type: "application/json" })); + } else { + fetch(URL_, { + method: "POST", body, keepalive: true, + headers: { "Content-Type": "application/json" }, + }).catch(() => {}); + } + } catch { /* instrumentation must never break the page */ } +} + +export function flush() { + clearTimeout(timer); + timer = 0; + if (!queue.length) return; + const items = queue; + queue = []; + send(items); +} + +/** Report one product event. `props` may carry only {view, prop, value}, all + * enums — see backend/core/events.py SCHEMA for the allowlists. */ +export function track(event, props) { + try { + if (!enabled() || !event) return; + queue.push(props ? { event, props } : { event }); + if (queue.length >= MAX_BATCH) { flush(); return; } + if (!timer) timer = setTimeout(flush, FLUSH_MS); + } catch { /* never break the page */ } +} + +// The four aggregate counters that already ship in production (metrics.EVENTS + +// home.nav_*). They predate the flag and predate the schema, and the ops +// endpoint's numbers must not silently reset when this file lands — so they keep +// their old unconditional, unbatched behaviour until they are migrated onto the +// schema above and the old names are retired. Everything NEW goes through +// track() and is flag-gated. +const LEGACY = /^home\.(locate|digest_signup|records_click|share|nav_[a-z]+)$/; + +export function trackLegacy(event) { + try { + if (optedOut || !event) return; + send([{ event }]); + } catch { /* never break the page */ } +} + +/** Which view this page is, for the `view` dimension. Derived from the path so + * no page has to declare it twice; the server re-checks it against VIEWS. */ +export function currentView() { + const p = location.pathname.replace(/\/+$/, ""); + const last = p.slice(p.lastIndexOf("/") + 1); + if (!last) return "home"; + if (["calendar", "day", "compare", "score", "legend", "privacy", "about"].includes(last)) return last; + if (last === "alerts") return "alerts"; + if (p.includes("/climate/")) return p.split("/climate/")[1].includes("/") ? "month" : "city"; + if (p.endsWith("/climate")) return "cities"; + if (p.includes("/glossary")) return "glossary"; + if (last === "records") return "records"; + return "home"; +} + +// pagehide covers the bfcache/navigation case; visibilitychange covers a tab +// switch or the phone locking. Both fire where beforeunload is unreliable. +addEventListener("pagehide", flush); +addEventListener("visibilitychange", () => { if (document.hidden) flush(); }); + +// Anything carrying data-event reports itself when activated. Optional +// data-event-prop / data-event-value carry the two extra dimensions, so a +// server-rendered link needs no JS of its own. +document.addEventListener("click", (e) => { + const el = e.target.closest?.("[data-event]"); + if (!el) return; + const d = el.dataset; + if (LEGACY.test(d.event)) { trackLegacy(d.event); return; } + track(d.event, { + view: d.eventView || currentView(), + prop: d.eventProp || "", + value: d.eventValue || "", + }); + // A click that navigates away must not lose its own event. + if (el.tagName === "A") flush(); +}); diff --git a/frontend/static/units.js b/frontend/static/units.js index 8a5689b..f326c10 100644 --- a/frontend/static/units.js +++ b/frontend/static/units.js @@ -159,8 +159,13 @@ function syncUnitToggle(wrap) { // Labelled for what it actually switches now — temperature, precipitation and // wind — even though the buttons read °F/°C. wrap.setAttribute("aria-label", "Units"); - wrap.innerHTML = '' - + ''; + // data-event/-prop/-value are picked up by track.js's global click delegation, + // so this file needs no import and no analytics code of its own. Which unit + // people actually switch TO is the number that would justify changing the + // locale-derived default. + wrap.innerHTML = + '' + + ''; wrap.addEventListener("click", (e) => { const b = e.target.closest("button[data-unit]"); if (b) setUnit(b.dataset.unit); diff --git a/frontend/templates/base.html.j2 b/frontend/templates/base.html.j2 index f8d825c..011bfea 100644 --- a/frontend/templates/base.html.j2 +++ b/frontend/templates/base.html.j2 @@ -1,5 +1,5 @@ - + diff --git a/frontend/tests/unit/test_pages.py b/frontend/tests/unit/test_pages.py index facfa78..53d8661 100644 --- a/frontend/tests/unit/test_pages.py +++ b/frontend/tests/unit/test_pages.py @@ -32,3 +32,37 @@ def test_old_static_index_is_gone(client): backend/tests/web/test_homepage.py, repo-split Stage 7a -- frontend's own StaticFiles mount is what actually answers this now).""" assert client.get(f"{B}/index.html").status_code == 404 + + +# --- product-event instrumentation flag --------------------------------------- +# track.js sends nothing unless it finds data-tg-events on . The whole +# rollout story rests on that stamp being absent by default, so assert both +# halves: off unless THERMOGRAPH_EVENTS is set, and applied to BOTH HTML paths +# (the Jinja-rendered content pages and the static SPA shells). + +def test_event_flag_is_off_by_default_on_every_html_path(client): + for path in ("/", "/calendar", "/day", "/score", "/compare", "/alerts"): + r = client.get(f"{B}{path}") + assert r.status_code == 200, path + assert "data-tg-events" not in r.text, path + + +def test_event_flag_stamps_both_html_paths_when_enabled(monkeypatch): + """Rebuilt in-process rather than reusing `client`: the flag is read at + import and the SPA shells memoize their prepped template.""" + import importlib + import content as content_mod + from fastapi.testclient import TestClient + + monkeypatch.setenv("THERMOGRAPH_EVENTS", "1") + content_mod = importlib.reload(content_mod) + assert content_mod.EVENTS_ENABLED + import app as app_mod + app_mod = importlib.reload(app_mod) + flagged = TestClient(app_mod.app) + for path in ("/calendar", "/compare"): + assert 'data-tg-events="1"' in flagged.get(f"{B}{path}").text, path + # Leave the modules as the rest of the suite expects to find them. + monkeypatch.delenv("THERMOGRAPH_EVENTS") + importlib.reload(content_mod) + importlib.reload(app_mod)