fix(analytics): correct channel attribution, crawler contamination, and period-blind overview

[dviejokfs]

Three independent analytics defects, all found by reading our own dashboard for the temps-landing-new project and being unable to reconcile the numbers.

1. Self-referrals were attributed to the "Referral" channel

record_event read the tracked site's hostname from event_data.hostname. No browser SDK has ever sent that key — the React analytics SDK sends a top-level domain, and EventMetricsPayload doesn't even deserialize it. So get_channel always got current_hostname = None, its self-referral branch never fired, and every internal page-to-page navigation was recorded as a Referral.

Production impact on temps.sh (30 days):

Events classified "Referral"	36,709 / 41,220 (89.1%)
Referrer breakdown total (self-referrals already filtered)	5,438
Largest genuine referrer (openalternative.co)	238

Channel attribution was unusable — real acquisition channels were buried under the site's own navigation.

The ingest handler already resolves the Host header against the route table (it has to, to attribute the event to a project), so the correct hostname was available all along. It's now threaded in as an explicit site_hostname argument and used for both self-referral detection and the hostname column — which until now stored the "localhost" fallback on every SDK-originated event.

The server-side console path passes None: its caller is an app backend, not a browser, and it sends no referrer.

Not fixed here: existing rows keep their stored channel. A backfill would rewrite the channel column across the whole events hypertable, which is a heavy operation on the reference 4 GB box — worth doing deliberately, not as a side effect of this PR.

2. Breakdowns counted crawlers; headline counts didn't

get_unique_counts filters is_crawler = false and its comment asserts the per-page and per-property queries do the same. They didn't. get_property_breakdown, get_property_timeline and get_page_paths all counted crawler events, so every percentage was computed over a bigger population than the totals shown next to it. The symptom was visible in the product: a literal Bot row (224) in the device breakdown and misc crawler (215) in browsers.

All three now filter crawlers. AI-agent and crawler reporting is unaffected — those views read proxy logs, not the events table.

3. analytics overview ignored --period in two places

Top Locations listed raw visitor rows and tallied countries client-side. /analytics/visitors caps at 100 rows (the command asked for 200) and orders by last_seen DESC, so 24h, 7d and 30d all got the same 100 most-recent visitors and printed identical tables:

# before — byte-identical for --period 24h, 7d and 30d
  1   United States      40   40.0%
  2   United Kingdom      7    7.0%

# after — 24h / 7d / 30d
  United States    71  26.7%   |   295  25.5%   |   1,307  27.7%
  Singapore        33  12.4%   |   167  14.4%   |     766  16.2%

The 30-day figures now reconcile with the headline 4,489 unique visitors. (Singapore at 16% is real, and worth a separate look.)

The sparkline always requested hourly buckets and kept the last 48, so any period beyond two days silently drew only its final two days under a header claiming the full range:

# before — same 48 chars under both "last 7 days" and "last 30 days"
  Hourly Visitors
  ▁▁▁▁▂▂▁▂▂▂▁▁▂▂▂▂▂▁▃▄▃▃▂▄▂▂▂▁▂▁▁▂▂▁▁▂▁▂▄▂▃█▁▂▃▂▂▁ (max: 44)

# after — bucket size chosen to fit the period, and labelled
  Visitors (last 24 hours, per 1 hour)
  ▂▂▃▂▂▂▂▂▂▁▁▂▁▂▄▂▃█▂▂▃▂▂▂▂ (max: 46)

  Visitors (last 7 days, per 6 hours)
  ▁▃▄▄▄▆▃▄▄▂▂▂▄▂▃▄▄▃▃▄▄▃▄▄▆▄▃█▅ (max: 106)

  Visitors (last 30 days, per 1 day)
  ▂▅▅▂▄▄▅▅▇▇█▆▅▆▆▆▅▆▅▅▅▅▅▅▇▅▃▅▅▇▆ (max: 248)

The label echoes the bucket size the server reports, so the caption always describes the data actually drawn.

Evidence

Regression tests (real Postgres via TestDatabase, skip gracefully without Docker):

• test_record_event_classifies_self_referral_as_direct — asserts a referrer on the site's own host yields Direct, a third-party referrer still yields Referral, and the resolved host is persisted instead of "localhost".
• test_property_breakdown_excludes_crawlers — asserts a bot-UA referrer is absent from the breakdown and excluded from its denominator.

The self-referral test was confirmed to fail on the pre-fix code with exactly the production symptom, then pass after:

$ cargo test --lib -p temps-analytics-events test_record_event_classifies_self_referral_as_direct
# with the fix reverted:
panicked at events_service.rs:3232:
assertion `left == right` failed: a referrer on the site's own host is session continuation, not a Referral
  left: Some("Referral")
 right: Some("Direct")

# with the fix in place:
test result: ok. 1 passed

Full suites:

$ cargo test --lib -p temps-analytics-events -p temps-analytics
82 passed (2 suites, 31.50s)

$ cargo clippy -p temps-analytics-events -p temps-analytics --all-targets -- -D warnings
Finished — no warnings

$ cargo check --lib          # whole workspace
Finished in 2m 04s

CLI verified against a live instance (app.temps.kfs.es, project temps-landing-new) — the before/after tables above are real output from bun run src/index.ts analytics overview at each period, not mock-ups. bunx tsc --noEmit introduces no new errors (5 pre-existing errors remain in openapi-ts.config.ts, notifications/, providers/, none in the changed file).

The backend changes can't be exercised against production until deployed, so they're evidenced by the DB-backed integration tests above.

Reviewer notes

• Behaviour change: breakdowns will drop visibly (the temps.sh 30-day event denominator falls from 41,220 toward the crawler-free population) and the "Bot"/"misc crawler" rows disappear. That's the point — those numbers previously couldn't be reconciled against the visitor and page-view totals beside them.
• Not addressed: sessions (10,529) exceeding page_views (7,564) is by definition, not a bug — sessions count distinct session_id across all events, page views count only event_type = 'page_view', and this site fires many non-pageview events. Flagging it because the overview renders the two adjacently, which invites the wrong reading.

[github-actions]

📓 Changelog preview

This is what your commits will add to the generated CHANGELOG.md at release time (via git-cliff). Do not edit CHANGELOG.md by hand — it is generated from your Conventional Commit messages.

## [Unreleased]

### Added

- **analytics:** Crawler opt-in for breakdowns, and prune NULL keys early

### Documentation

- **analytics:** Declare include_crawlers on the timeline endpoint

### Fixed

- **analytics:** Classify self-referrals as Direct, not Referral
- **analytics:** Exclude crawler traffic from breakdowns and top pages
- **cli:** Make analytics overview locations and sparkline respect --period
- **analytics:** Populate visitor language instead of always "Unknown"
- **analytics:** Send language from the shared browser SDK core too
- **analytics:** Close review findings on attribution, language and CH parity

[dviejokfs]

Added: end-to-end verification through the real ingest path, and a fourth fix

The wiring gap is now closed

The original evidence tested record_event directly, which verified the service does the right thing given a hostname but assumed the handler supplied the right one. That assumption is now executed rather than inspected.

Set up on a local instance (own slot, own database): a project, a custom domain, and a deployment so the host resolves in the route table. Then real POST /api/_temps/event requests through the proxy with varying Referer, reading back the persisted columns:

Referer sent	channel recorded
attribution-test.localho.st (own host)	Direct	✅ was Referral before the fix
openalternative.co	Referral	✅
www.google.com	Organic Search	✅
(none)	Direct	✅

hostname persisted as attribution-test.localho.st on all four, not the "localhost" fallback. This exercises Host-header resolution → route table → handler → service, which no unit test covered.

Fourth fix: language was 100% "Unknown"

Found while looking at the same dashboard:

Languages — temps-landing-new (last 30 days)
  1   Unknown     41,437   100.0%
  2   en-US            1     0.0%

Identical failure shape to the hostname bug — the server only read language from the payload and event_data, and no browser SDK ever sends it there (the React SDK reports navigator.language from the session recorder only, never from trackEvent/trackPageview). Meanwhile every browser sends Accept-Language on the ingest request itself, so the data was always present and simply never read.

Fixed on both sides: the server falls back to the header (repairs the dimension for every already-deployed SDK, no upgrade needed), and the SDK now sends navigator.language on all three event paths (more precise, since proxies can rewrite the header).

The header value is validated, not just truncated. language is a GROUP BY dimension, so accepting arbitrary header content would let any client mint unbounded distinct values — a cardinality bomb on a box expected to run in 4 GB. Verified against hostile input on the live instance:

Accept-Language sent	stored
en-US,en;q=0.9	en-US
es-ES,es;q=0.9,en;q=0.8	es-ES
pt-BR,pt;q=0.9	pt-BR
zh-Hant-TW,zh;q=0.9	zh-Hant-TW
*	(NULL)
'; DROP TABLE events--	(NULL)
200-char string	(NULL)

Read back through the public breakdown API, so this is the user-facing surface, not just the column:

=== language ===          === channel ===
  Unknown       7  63.6%    Direct          9  81.8%
  en-US         1   9.1%    Referral        1   9.1%
  pt-BR         1   9.1%    Organic Search  1   9.1%
  zh-Hant-TW    1   9.1%
  es-ES         1   9.1%

The 7 Unknown reconcile exactly: the 4 attribution events sent before the language fix (no Accept-Language) plus the 3 deliberately-rejected junk values.

Two unit tests cover the parser (test_primary_accept_language_takes_highest_priority_tag, test_primary_accept_language_rejects_junk). Suite is now 59 passing; clippy clean with -D warnings.

Still not addressed

Existing rows keep their stored channel and NULL language — all four fixes are forward-only. A backfill would rewrite two columns across the whole events hypertable and should be a deliberate, separately-reviewed operation rather than a side effect of this PR.

[dviejokfs]

Added since the original description: crawler opt-in, an exact perf win, and an OpenAPI fix

The description above covers the first three fixes. Three more landed since; recording them here so the PR is self-describing.

include_crawlers opt-in (resolves the open design question)

The earlier commit excluded crawlers from breakdowns unconditionally. That fixed the denominator mismatch but left the product inconsistent — the visitors list has a humans / crawlers / all filter, while breakdowns could never show bot traffic at all, with no affordance explaining why.

Breakdowns and timelines now take the same include_crawlers opt-in. Default stays false (the invariant: percentages must share the headline counts' denominator). Applied to both backends so they report one population.

The flag is set via a with_crawlers() builder rather than an 8th positional argument, matching the existing with_environment() / with_deployment() idiom — and avoiding a bare positional bool at every call site.

Verified both directions on Postgres (test_property_breakdown_excludes_crawlers) and on ClickHouse against a real container:

assert_eq!(off_total, 1, "default must count only the human visitor");
assert_eq!(on_total,  2, "opt-in must also count the crawler visitor");

The ClickHouse assertion matters beyond the feature: that driver binds positionally, so those two totals are what prove the new .bind(crawler_flag) lines up with its (? = 1 OR is_crawler = 0) placeholder. A misaligned bind would shift every later filter and silently change results.

Exact ~20% speedup, no approximation

Rows whose DISTINCT key is NULL were carried into the GROUP BY and then discarded by HAVING COUNT(...) > 0. COUNT(DISTINCT x) ignores NULLs anyway, so filtering them in WHERE returns an identical result set while skipping the sort work.

Measured on a 300k-event range where a third of rows carry no visitor_id:

	execution time (3 runs)
before	159 / 177 / 164 ms
after	129 / 135 / 142 ms

No approximation, no new index, no extension dependency.

OpenAPI: include_crawlers was invisible on the timeline endpoint

The timeline handler read the parameter but never declared it in #[utoipa::path(params(...))], so it worked over raw HTTP while being absent from the OpenAPI document and therefore from every generated client. Fixed in 5aa8e0b3.

Two things deliberately NOT done

Approximate distinct counting (HLL). COUNT(DISTINCT) is the wrong tool at scale and this repo already knows it — clickhouse_backend.rs uses uniq() and documents it as "within ~1%". Measured alternatives on the same 300k rows: HLL on raw rows ~50 ms (4×), HLL rolled up from a pre-aggregate ~17 ms (11×), with 0.00% error across 20 groups.

It is blocked, not deferred: timescaledb_toolkit is not a guaranteed dependency — migrations create only timescaledb and vector. It ships in the timescaledb-ha image but a self-hoster on plain Postgres + timescaledb would hit a hard failure on approx_count_distinct. Adopting it is a dependency decision, not a code change.

Regenerating the typed SDKs for the two new query params. Additive and optional, so nothing breaks, but the toggle isn't reachable from the generated clients until someone regenerates.

Unrelated finding worth its own issue

events_hourly stores count(DISTINCT visitor_id) per hour. Per-bucket distinct counts cannot be summed, so that CAGG cannot answer "unique visitors over 30 days" at all. Nothing currently reads it, so this is dormant rather than a live over-count — but it is refreshing on ingest for data no one queries.