From 39d874a7e22eb8061dc07077894692e8a0a20bdd Mon Sep 17 00:00:00 2001 From: Paperclip Agent Date: Thu, 29 Oct 2026 17:02:39 +0000 Subject: [PATCH] analytics: make each weekly funnel stage separately measurable (FRE-366) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gap 1 — click and served share one label: trackDownload now takes a stage ("click" | "served") carried on a new VisitorAnalytics.stage column; the Nook door and the tRPC proxy record clicks, the DMG file route records served. summariseDownloads() and the /analytics downloads block are untouched — the stage is a separate column, so the path grouping cannot change. Gap 2 — checkout start records nothing: the checkout route records a created Stripe session through the shared trackRequest writer (same referrer, campaign, country dimensions as every door) under its own path, and STAGES.checkout reads it instead of the /checkout page proxy. Gap 3 — loopback addresses, no host dimension: every writer now resolves the client address through resolveClientIp, which trusts x-forwarded-for only when the direct peer is the loopback production proxy (trust boundary stated in request-client.ts), and records the host on each row through the same site-context extractor routing uses. stage and host reach the table through VISITOR_ANALYTICS_ADDED_COLUMNS; the weekly job applies the same bootstrap before reading so it renders seven stages against a database no deploy has migrated yet. The weekly read splits downloadClick/dmgServed, scopes visits to nook.freno.me, and computes its caveats from the window: pre-instrumentation rows are stated and excluded, and the loopback warning now fires only when rows recorded since the instrumentation still carry it. --- marketing/metrics/weekly.md | 28 +-- scripts/weekly-metrics.ts | 10 +- src/db/types.ts | 2 + src/routes/api/downloads/[filename].ts | 2 +- src/routes/api/the-nook/checkout.ts | 18 ++ src/routes/api/the-nook/download.test.ts | 4 + src/routes/api/the-nook/download.ts | 8 +- src/server/analytics-schema.test.ts | 12 +- src/server/analytics-schema.ts | 18 +- src/server/analytics.ts | 10 +- src/server/api/routers/analytics.ts | 10 +- src/server/api/routers/downloads.ts | 2 +- src/server/api/utils.ts | 11 +- src/server/request-client.test.ts | 78 +++++++ src/server/request-client.ts | 67 ++++++ src/server/site-context-server.ts | 5 +- src/server/track-download.ts | 62 ++++- src/server/weekly-metrics.test.ts | 232 +++++++++++++----- src/server/weekly-metrics.ts | 285 +++++++++++++---------- 19 files changed, 643 insertions(+), 221 deletions(-) create mode 100644 src/server/request-client.test.ts create mode 100644 src/server/request-client.ts diff --git a/marketing/metrics/weekly.md b/marketing/metrics/weekly.md index 2ea9eaf..12b1f47 100644 --- a/marketing/metrics/weekly.md +++ b/marketing/metrics/weekly.md @@ -1,6 +1,6 @@ # Weekly funnel snapshot -Window: **2026-10-22 16:00:00 → 2026-10-29 16:00:00 UTC** (7 days). Previous: 2026-10-15 16:00:00 → 2026-10-22 16:00:00 UTC. +Window: **2026-10-22 17:01:33 → 2026-10-29 17:01:33 UTC** (7 days). Previous: 2026-10-15 17:01:33 → 2026-10-22 17:01:33 UTC. Generated by `bun --env-file=.env scripts/weekly-metrics.ts`; regenerated weekly, never hand-edited. The analysis is the CMO's. @@ -8,10 +8,11 @@ Generated by `bun --env-file=.env scripts/weekly-metrics.ts`; regenerated weekly | Stage | This week | Last week | Change | |---|---:|---:|---:| -| visit (page views) | 61 | 56 | +5 | -| download click → DMG served (one label) † | 156 | 0 | +156 | +| visit (nook.freno.me page views) | 0 | 0 | 0 | +| download click | 0 | 0 | 0 | +| DMG served | 0 | 0 | 0 | | trial started | 2 | 0 | +2 | -| checkout page visit † | 0 | 0 | 0 | +| checkout started (Stripe session) | 0 | 0 | 0 | | purchase (licence issued) | 0 | 0 | 0 | | activation | 0 | 0 | 0 | @@ -21,31 +22,22 @@ Top 10 by volume, untagged excluded — the same grouping `getAnalyticsSummary.t | Source | Medium | Campaign | Rows | |---|---|---|---:| -| hn | post | launch | 2 | -| direct | — | deploycheck-direct | 1 | -| hn | — | flushproof | 1 | -| hn | post | deploycheck-ref | 1 | -| r_macapps | — | week2 | 1 | | x | — | — | 1 | -7 of 356 rows in the window carry campaign tags; the rest are untagged and therefore unattributed. +1 of 291 rows in the window carry campaign tags; the rest are untagged and therefore unattributed. ## The one thing that moved -**download click → DMG served (one label): 0 → 156 (+156).** +**trial started: 0 → 2 (+2).** ## Before acting on these numbers -- **356 of 356 rows in the window came from a loopback address** (127.0.0.1 / ::1); every one of the 12035 addressed rows the table has ever held is loopback (13 carry no address). Either this host is the only audience, or the app sits behind a local proxy and `getRequestIP` records the proxy, not the visitor. Either way the address identifies nobody — "unique visitors" collapses to one host and per-channel reach cannot be read from it. -- **The download stage is one label, not two.** The landing-door route and the file route that serves the DMG both write `/api/downloads/thenook`, so "download click → DMG served" cannot be split: one served click writes two rows, a click that fails to resolve writes one, and a Sparkle update writes one. Of this window's 156 download rows, 156 are loopback and 117 are not browser user agents (curl, Sparkle, link-preview crawlers). -- **Checkout start is not recorded.** `POST /api/the-nook/checkout` — the call that actually creates the Stripe session — writes nothing to analytics; the only trace is the /checkout page view reported above, which is a page, not a started checkout. -- **There is no host dimension.** `VisitorAnalytics` records a path, not the site, so the Nook's landing page (`/` on nook.freno.me) is the same row as freno.me's root. "visit" above is site-wide page views, not Nook visits. +- **291 of 291 rows in the window predate the FRE-366 instrumentation.** They carry no host, and their recorded address is the production proxy's loopback (127.0.0.1 / ::1) — the writers then recorded the direct peer, not the visitor. They are excluded from every host-scoped stage above (visits, and the download click/served split), and "unique visitors" over them still collapses to one host. Rows recorded since carry the real client address and the site they addressed. +- **Download rows are automation-heavy.** Of this window's 137 download rows, 0 are clicks and 0 are served files; the remaining 137 predate the click/served stage label and cannot be split, so they count in neither stage. 112 of all 137 are not browser user agents (curl, Sparkle, link-preview crawlers). - **No conversion rate before the cohort matures.** Trial→paid belongs to the mature Day 28–35 read (FRE-197); a ratio from a partial cohort is not a metric. -† Proxy or merged label — see *Before acting on these numbers* above. - ## Sources - `VisitorAnalytics` (freno.me Turso DB) — visits, downloads, campaigns. - `trials`, `licenses`, `activations` (the-nook Turso DB) — the paid funnel. -- Generated 2026-10-29T16:00:00Z. +- Generated 2026-10-29T17:01:33Z. diff --git a/scripts/weekly-metrics.ts b/scripts/weekly-metrics.ts index f24c693..c670ac7 100644 --- a/scripts/weekly-metrics.ts +++ b/scripts/weekly-metrics.ts @@ -2,6 +2,7 @@ import { createClient } from "@libsql/client/web"; import { mkdir, writeFile } from "node:fs/promises"; import { dirname, resolve } from "node:path"; import { + ensureAnalyticsSchema, getWeeklyReport, renderWeeklyMarkdown, type Connections @@ -123,6 +124,11 @@ async function main(): Promise { nook: connect(requiredEnv("NOOK_DB_URL"), requiredEnv("NOOK_DB_TOKEN")) }; + // The app migrates the table on boot, but this job must not depend on a + // deploy having happened: add any missing post-hoc columns (stage, host) + // through the same idempotent bootstrap path first. + await ensureAnalyticsSchema(connections.analytics); + const report = await getWeeklyReport(connections, { days: options.days, now: options.now @@ -141,8 +147,8 @@ async function main(): Promise { console.log( `wrote ${options.out}\n` + `window ${report.window.start} → ${report.window.end}\n` + - `visit ${f.visit}, download ${f.download}, trial ${f.trial}, ` + - `checkout ${f.checkout}, purchase ${f.purchase}, activation ${f.activation}\n` + + `visit ${f.visit}, download click ${f.downloadClick}, DMG served ${f.dmgServed}, ` + + `trial ${f.trial}, checkout ${f.checkout}, purchase ${f.purchase}, activation ${f.activation}\n` + `mover: ${report.mover ? `${report.mover.label} ${report.mover.before} → ${report.mover.now}` : "none"}` ); } diff --git a/src/db/types.ts b/src/db/types.ts index acd1a4f..7276f39 100644 --- a/src/db/types.ts +++ b/src/db/types.ts @@ -160,6 +160,8 @@ export interface VisitorAnalytics { ttfb?: number | null; dom_load?: number | null; load_complete?: number | null; + stage?: string | null; + host?: string | null; created_at: string; } diff --git a/src/routes/api/downloads/[filename].ts b/src/routes/api/downloads/[filename].ts index 89496b6..8e210e7 100644 --- a/src/routes/api/downloads/[filename].ts +++ b/src/routes/api/downloads/[filename].ts @@ -95,7 +95,7 @@ export async function GET(event: APIEvent) { console.log(`✓ Serving ${filename} (${body.length} bytes)`); - await trackDownload(event, assetKeys[asset]!); + await trackDownload(event, assetKeys[asset]!, "served"); // SAFETY: `transformToByteArray` yields `Uint8Array`, which the // DOM lib's `BufferSource` (ArrayBuffer-backed) does not accept, though undici diff --git a/src/routes/api/the-nook/checkout.ts b/src/routes/api/the-nook/checkout.ts index 561dbb4..478755c 100644 --- a/src/routes/api/the-nook/checkout.ts +++ b/src/routes/api/the-nook/checkout.ts @@ -3,6 +3,7 @@ import { env } from "~/env/server"; import { TURNSTILE_CONFIG } from "~/config"; import { verifyTurnstileToken } from "~/server/fetch-utils"; import { json, error } from "./_lib"; +import { trackRequest } from "~/server/track-download"; /** * POST /api/the-nook/checkout @@ -12,6 +13,14 @@ import { json, error } from "./_lib"; * session for the one-time $10 The Nook license. Returns the hosted * checkout URL for the client to redirect to. * + * Analytics: a created session is the funnel's "checkout started" stage + * (FRE-366). Until then the only trace was the client-side /checkout page + * view — a page, not a started checkout. Recorded through the shared + * `trackRequest` writer (referrer, campaign tags, country, real client + * address, host) under this route's own path, best-effort: a failed + * analytics write never blocks a checkout, and an attempt Turnstile or + * Stripe rejects is correctly not a started checkout. + * * Uses raw `fetch` (Node 24 / Vercel Node functions have global fetch) — no * `stripe` npm dependency added. */ @@ -67,5 +76,14 @@ export async function POST(event: APIEvent) { if (!data.url) { return error("Stripe returned no checkout URL", 502); } + + // The session exists: that is "checkout started". Awaited so the row is + // flushed before the response — a buffered write is not guaranteed to + // survive the response on serverless. + await trackRequest(event, { + path: "/api/the-nook/checkout", + method: "POST" + }); + return json({ checkoutUrl: data.url }); } diff --git a/src/routes/api/the-nook/download.test.ts b/src/routes/api/the-nook/download.test.ts index a230002..026a77f 100644 --- a/src/routes/api/the-nook/download.test.ts +++ b/src/routes/api/the-nook/download.test.ts @@ -113,6 +113,10 @@ describe("GET /api/the-nook/download", () => { expect(logged[0]).toMatchObject({ path: "/api/downloads/thenook", method: "GET", + // The door is the click half of the split; the file route writes + // "served". And the row names the site it came from (FRE-366). + stage: "click", + host: "nook.freno.me", referrer: "https://nook.freno.me/" }); // Flushed within the request, not left to the batch timer. diff --git a/src/routes/api/the-nook/download.ts b/src/routes/api/the-nook/download.ts index 6d0a1ee..13479b0 100644 --- a/src/routes/api/the-nook/download.ts +++ b/src/routes/api/the-nook/download.ts @@ -13,8 +13,10 @@ import { resolveLatestDownload } from "./_download"; * Why a redirect and not a direct S3 link: the appcast route and the DMG * route never logged anything, so step 2 of the funnel (download click → * DMG served) was invisible. Every click now lands one row in - * VisitorAnalytics under `/api/downloads/thenook`, alongside the clicks and - * served files of every other product — see `~/server/track-download`. + * VisitorAnalytics under `/api/downloads/thenook` with `stage = "click"`, + * distinct from the `stage = "served"` row the file route writes when the + * DMG is actually streamed — the split FRE-366 added, alongside the clicks + * and served files of every other product — see `~/server/track-download`. * * Referrer, campaign tags, edge country (with `?c=` override) and user agent * are read by `trackDownload`, which is shared with the other download doors. @@ -28,7 +30,7 @@ import { resolveLatestDownload } from "./_download"; export async function GET(event: APIEvent): Promise { // Recorded before resolving: a click that dies at resolution is exactly the // failure the funnel needs to show. - await trackDownload(event, "thenook"); + await trackDownload(event, "thenook", "click"); let target: string | null = null; try { diff --git a/src/server/analytics-schema.test.ts b/src/server/analytics-schema.test.ts index 70c99dd..787dfc6 100644 --- a/src/server/analytics-schema.test.ts +++ b/src/server/analytics-schema.test.ts @@ -40,7 +40,9 @@ const sampleArgs = [ 2, // inp 3, // ttfb 4, // dom_load - 5 // load_complete + 5, // load_complete + "click", // stage + "nook.freno.me" // host ]; beforeEach(() => { @@ -69,7 +71,7 @@ describe("VisitorAnalytics schema", () => { } // The reverse direction matters too: a column the code writes but the // DDL omits is the failure this whole file exists to catch. - expect(VISITOR_ANALYTICS_COLUMNS.length).toBe(23); + expect(VISITOR_ANALYTICS_COLUMNS.length).toBe(25); }); it("has one placeholder per inserted column", () => { @@ -83,7 +85,7 @@ describe("VisitorAnalytics schema", () => { const row = db .prepare( - `SELECT path, country, utm_source, utm_medium, utm_campaign + `SELECT path, country, utm_source, utm_medium, utm_campaign, stage, host FROM VisitorAnalytics` ) .get() as Record; @@ -93,7 +95,9 @@ describe("VisitorAnalytics schema", () => { country: "GB", utm_source: "hn", utm_medium: "post", - utm_campaign: "launch" + utm_campaign: "launch", + stage: "click", + host: "nook.freno.me" }); }); diff --git a/src/server/analytics-schema.ts b/src/server/analytics-schema.ts index 93d43ba..19119cf 100644 --- a/src/server/analytics-schema.ts +++ b/src/server/analytics-schema.ts @@ -51,6 +51,8 @@ export const VISITOR_ANALYTICS_DDL = ` utm_source TEXT, utm_medium TEXT, utm_campaign TEXT, + stage TEXT, + host TEXT, FOREIGN KEY (user_id) REFERENCES User(id) ON DELETE SET NULL ) `; @@ -79,6 +81,18 @@ export const VISITOR_ANALYTICS_ADDED_COLUMNS = [ { name: "utm_campaign", ddl: `ALTER TABLE VisitorAnalytics ADD COLUMN utm_campaign TEXT` + }, + { + // Which funnel stage a download row is: "click" (a door) or "served" + // (the file route). Null on every non-download row (FRE-366). + name: "stage", + ddl: `ALTER TABLE VisitorAnalytics ADD COLUMN stage TEXT` + }, + { + // The Host the client addressed, e.g. "nook.freno.me" — the site + // dimension the table never had (FRE-366). + name: "host", + ddl: `ALTER TABLE VisitorAnalytics ADD COLUMN host TEXT` } ]; @@ -110,7 +124,9 @@ export const VISITOR_ANALYTICS_COLUMNS = [ "inp", "ttfb", "dom_load", - "load_complete" + "load_complete", + "stage", + "host" ] as const; /** diff --git a/src/server/analytics.ts b/src/server/analytics.ts index 198925e..b37cef0 100644 --- a/src/server/analytics.ts +++ b/src/server/analytics.ts @@ -64,6 +64,10 @@ export interface AnalyticsEntry { ttfb?: number | null; domLoad?: number | null; loadComplete?: number | null; + /** Funnel stage for download rows ("click" | "served"); null elsewhere. */ + stage?: string | null; + /** Host the client addressed, e.g. "nook.freno.me" — the site dimension. */ + host?: string | null; } /** @@ -124,7 +128,9 @@ async function flushAnalyticsBuffer(): Promise { entry.inp || null, entry.ttfb || null, entry.domLoad || null, - entry.loadComplete || null + entry.loadComplete || null, + entry.stage || null, + entry.host || null ] }); } @@ -258,6 +264,8 @@ export async function queryAnalytics( utm_medium: row.utm_medium as string | null, utm_campaign: row.utm_campaign as string | null, duration_ms: row.duration_ms as number | null, + stage: row.stage as string | null, + host: row.host as string | null, created_at: row.created_at as string })); } diff --git a/src/server/api/routers/analytics.ts b/src/server/api/routers/analytics.ts index 3cd6a5e..e5049f3 100644 --- a/src/server/api/routers/analytics.ts +++ b/src/server/api/routers/analytics.ts @@ -10,7 +10,7 @@ import { } from "~/server/analytics"; import { ConnectionFactory } from "~/server/database"; import { v4 as uuid } from "uuid"; -import { getRequestIP } from "vinxi/http"; +import { resolveClientIp, requestHost } from "~/server/request-client"; import { parseUtm } from "~/server/utm"; import { VISITOR_ANALYTICS_INSERT } from "~/server/analytics-schema"; @@ -98,7 +98,8 @@ export const analyticsRouter = createTRPCRouter({ getHeader(req.headers, "referer") || getHeader(req.headers, "referrer") || getHeader(ctx.event.request?.headers, "referer"); - const ipAddress = getRequestIP(ctx.event.nativeEvent) || undefined; + const ipAddress = resolveClientIp(ctx.event) || undefined; + const host = requestHost(ctx.event) || undefined; const enriched = enrichAnalyticsEntry({ userId: ctx.userId, path: input.path, @@ -106,6 +107,7 @@ export const analyticsRouter = createTRPCRouter({ userAgent, referrer, ipAddress, + host, // The client beacon sends `pathname + search`, so the page URL's // own query string is the campaign source; the referrer fills any // gap (e.g. clicks that arrive on a clean path). @@ -145,7 +147,9 @@ export const analyticsRouter = createTRPCRouter({ enriched.inp || null, enriched.ttfb || null, enriched.domLoad || null, - enriched.loadComplete || null + enriched.loadComplete || null, + enriched.stage || null, + enriched.host || null ] }); diff --git a/src/server/api/routers/downloads.ts b/src/server/api/routers/downloads.ts index fd79ed1..e053c0f 100644 --- a/src/server/api/routers/downloads.ts +++ b/src/server/api/routers/downloads.ts @@ -79,7 +79,7 @@ export const downloadsRouter = createTRPCRouter({ // exactly the event the funnel needs to show. Unknown asset names are // skipped rather than recorded as a download that never happened. if (assets[input.asset_name] || latestAssets[input.asset_name]) { - await trackDownload(ctx.event, input.asset_name); + await trackDownload(ctx.event, input.asset_name, "click"); } const bucket = env.VITE_DOWNLOAD_BUCKET_STRING; diff --git a/src/server/api/utils.ts b/src/server/api/utils.ts index 60ed1e9..7d2fbb8 100644 --- a/src/server/api/utils.ts +++ b/src/server/api/utils.ts @@ -1,7 +1,7 @@ import { initTRPC, TRPCError } from "@trpc/server"; import type { APIEvent } from "@solidjs/start/server"; import { logVisit, enrichAnalyticsEntry } from "~/server/analytics"; -import { getRequestIP } from "vinxi/http"; +import { resolveClientIp, requestHost } from "~/server/request-client"; import { getAuthPayloadFromEvent } from "~/server/auth"; export type Context = { @@ -51,7 +51,11 @@ async function createContextInner(event: APIEvent): Promise { getHeader(req.headers, "referer") || getHeader(req.headers, "referrer") || getHeader(event.request?.headers, "referer"); - const ipAddress = getRequestIP(event.nativeEvent) || undefined; + // Behind the production proxy the direct peer is loopback, so the real + // client address comes from the forwarded header the proxy sets — see + // `~/server/request-client` for the trust boundary (FRE-366). + const ipAddress = resolveClientIp(event) || undefined; + const host = requestHost(event) || undefined; // Don't log the performance logging endpoint itself to avoid circular tracking if (!path.includes("analytics.logPerformance")) { @@ -62,7 +66,8 @@ async function createContextInner(event: APIEvent): Promise { method, userAgent, referrer, - ipAddress + ipAddress, + host }) ); } diff --git a/src/server/request-client.test.ts b/src/server/request-client.test.ts new file mode 100644 index 0000000..fc40bb8 --- /dev/null +++ b/src/server/request-client.test.ts @@ -0,0 +1,78 @@ +import { beforeEach, describe, expect, it, mock } from "bun:test"; + +// The trust boundary of FRE-366: x-forwarded-for is client-controlled text, +// believed only when the direct peer is the loopback production proxy. These +// tests pin that boundary — a regression here either re-records the proxy as +// the visitor (unique visitors collapses to one host) or lets any client +// spoof its address. + +let directIp: string | null = null; +mock.module("vinxi/http", () => ({ + getRequestIP: () => directIp +})); + +const { resolveClientIp, isLoopbackAddress, requestHost } = await import( + "./request-client" +); + +function event( + headers: Record = {}, + url = "https://nook.freno.me/downloads" +) { + return { + request: new Request(url, { headers }), + nativeEvent: {} + } as never; +} + +beforeEach(() => { + directIp = null; +}); + +describe("resolveClientIp", () => { + it("takes the first forwarded hop when the direct peer is the loopback proxy", () => { + directIp = "127.0.0.1"; + expect( + resolveClientIp(event({ "x-forwarded-for": "198.51.100.9, 10.0.0.1" })) + ).toBe("198.51.100.9"); + + directIp = "::1"; + expect( + resolveClientIp(event({ "x-forwarded-for": "198.51.100.10" })) + ).toBe("198.51.100.10"); + }); + + it("ignores a forwarded header from a real peer — it is spoofable", () => { + directIp = "203.0.113.7"; + expect( + resolveClientIp(event({ "x-forwarded-for": "198.51.100.9" })) + ).toBe("203.0.113.7"); + }); + + it("keeps the loopback peer when the proxy set no forwarded header", () => { + directIp = "::ffff:127.0.0.1"; + expect(resolveClientIp(event())).toBe("::ffff:127.0.0.1"); + }); + + it("records nothing rather than inventing an address", () => { + expect(resolveClientIp(event())).toBeNull(); + }); +}); + +describe("isLoopbackAddress", () => { + it("covers the v4 range, the shorthand and the v6 forms", () => { + expect(isLoopbackAddress("127.0.0.1")).toBe(true); + expect(isLoopbackAddress("127.0.0.53")).toBe(true); + expect(isLoopbackAddress("::1")).toBe(true); + expect(isLoopbackAddress("::ffff:127.0.0.1")).toBe(true); + expect(isLoopbackAddress("203.0.113.7")).toBe(false); + expect(isLoopbackAddress("10.0.0.1")).toBe(false); + }); +}); + +describe("requestHost", () => { + it("reads the host the client addressed from the request URL", () => { + expect(requestHost(event())).toBe("nook.freno.me"); + expect(requestHost(event({}, "https://freno.me/"))).toBe("freno.me"); + }); +}); diff --git a/src/server/request-client.ts b/src/server/request-client.ts new file mode 100644 index 0000000..2d79080 --- /dev/null +++ b/src/server/request-client.ts @@ -0,0 +1,67 @@ +import type { APIEvent } from "@solidjs/start/server"; +import { getRequestIP } from "vinxi/http"; +import { hostFromEventLike } from "./site-context-server"; + +/** + * The real client address behind the production proxy (FRE-366). + * + * Until this existed every analytics writer called `getRequestIP` and only + * looked at `x-forwarded-for` when it returned null — so behind the + * production proxy the recorded address was the proxy itself, and all + * 12,035 addressed rows in VisitorAnalytics carried 127.0.0.1 / ::1. + * + * Trust boundary, stated plainly: `x-forwarded-for` is client-controlled + * text. It is believed only when the direct peer is a loopback address — + * the production shape, where a same-host proxy (nginx/caddy) terminates + * TLS and sets the header itself. Anywhere else the direct peer IS the + * client (dev, direct hit), and a forwarded header from it is ignored as + * spoofable. The first hop of the header is taken; later hops are the + * client's own claims. + */ +export function resolveClientIp(event: APIEvent): string | null { + const direct = getRequestIP(event.nativeEvent); + + if (direct && !isLoopbackAddress(direct)) { + return direct; + } + + const forwarded = event.request?.headers?.get?.("x-forwarded-for"); + const first = forwarded?.split(",")[0]?.trim(); + if (first) { + return first; + } + + return direct ?? null; +} + +export function isLoopbackAddress(ip: string): boolean { + return ( + ip === "::1" || + ip === "::ffff:127.0.0.1" || + ip === "127.0.0.1" || + ip.startsWith("127.") + ); +} + +/** + * The site the request addressed — the `Host` the client asked for, e.g. + * `nook.freno.me`. Recorded on every analytics row so a weekly read can + * scope to one site; without it `/` on nook.freno.me is the same row as + * `/` on freno.me. Thin alias over the site-context extractor so the + * analytics writers share one host-parsing implementation with routing. + */ +export function requestHost(event: APIEvent): string | null { + const header = hostFromEventLike(event); + if (header) return header; + + // `host` is a forbidden header and some server shapes drop it from + // `event.request.headers`; the request URL is absolute on the server and + // carries the same host the client addressed. + try { + const url = event.request?.url; + if (url) return new URL(url).host || null; + } catch { + /* noop */ + } + return null; +} diff --git a/src/server/site-context-server.ts b/src/server/site-context-server.ts index 21a272e..fc694b6 100644 --- a/src/server/site-context-server.ts +++ b/src/server/site-context-server.ts @@ -38,9 +38,10 @@ export interface ServerSiteEvent { * the raw Node request via `event.nativeEvent.node.req.headers`. We try * both so this works for API route handlers (`APIEvent`), the SSR document * handler (`PageEvent`), and tRPC procedures operating on the underlying - * `H3Event`. + * `H3Event`. Also used by the analytics writers to record the site dimension + * (`host` column) on every row (FRE-366). */ -function hostFromEventLike(event: ServerSiteEvent | unknown): string | null { +export function hostFromEventLike(event: ServerSiteEvent | unknown): string | null { // 1) Web FetchEvent / APIEvent / PageEvent → standard `Request` headers. try { const req = (event as { request?: Request } | null)?.request; diff --git a/src/server/track-download.ts b/src/server/track-download.ts index 533186d..7435bd8 100644 --- a/src/server/track-download.ts +++ b/src/server/track-download.ts @@ -1,5 +1,4 @@ import type { APIEvent } from "@solidjs/start/server"; -import { getRequestIP } from "vinxi/http"; import { logVisit, enrichAnalyticsEntry, @@ -7,6 +6,7 @@ import { } from "~/server/analytics"; import { parseUtm } from "~/server/utm"; import { downloadPath } from "~/lib/download-asset"; +import { resolveClientIp, requestHost } from "~/server/request-client"; /** * Record one download event under the canonical path `/api/downloads/`. @@ -15,35 +15,75 @@ import { downloadPath } from "~/lib/download-asset"; * label per asset to group on: the tRPC click proxy, the S3 file route that * serves Sparkle updates, and the Nook door. * + * The `stage` is what separates the doors within one asset label: "click" + * for a door the visitor chose (landing button, tRPC proxy), "served" for + * the file route that actually streams the DMG. Until FRE-366 both wrote the + * same unlabelled row, so "download click → DMG served" could not be split — + * one served click wrote two identical rows. `summariseDownloads()` and the + * /analytics downloads block still group on the path alone; the stage is a + * separate column precisely so that grouping cannot change. + * * Best-effort by design — a failed analytics write must never block a * download. Flushed rather than merely buffered: a download is low-volume and * the 30s batch timer is not guaranteed to survive the response on serverless. + */ +export async function trackDownload( + event: APIEvent, + asset: string, + stage: DownloadStage +): Promise { + await trackRequest(event, { + path: downloadPath(asset), + method: "GET", + stage + }); +} + +export type DownloadStage = "click" | "served"; + +export interface TrackedRequest { + path: string; + method: string; + /** Funnel stage label; only download rows carry one today. */ + stage?: string | null; +} + +/** + * Record one server-side request event with the shared visitor dimensions: + * referrer, campaign tags, edge country, real client address and host. + * + * This is the one writer every measured door goes through — download doors + * via `trackDownload`, the checkout door directly — so the dimensions can + * never drift between doors. Same best-effort, flushed-immediately contract + * as `trackDownload`. * * `event.request` is expected to be a Fetch `Request` (true for Vinxi route * handlers and browser tRPC calls). A server-side `createCaller` hands over a * Node request, whose headers have no `get` — that throw is swallowed below, - * so such a click is not recorded. No server-side caller downloads today. + * so such an event is not recorded. No server-side caller downloads today. */ -export async function trackDownload( +export async function trackRequest( event: APIEvent, - asset: string + tracked: TrackedRequest ): Promise { const { request } = event; try { const referrer = request.headers.get("referer") ?? request.headers.get("referrer"); - const forwarded = request.headers.get("x-forwarded-for"); await logVisit( enrichAnalyticsEntry({ - path: downloadPath(asset), - method: "GET", + path: tracked.path, + method: tracked.method, + stage: tracked.stage ?? null, referrer, userAgent: request.headers.get("user-agent"), - ipAddress: - getRequestIP(event.nativeEvent) ?? - (forwarded ? forwarded.split(",")[0]!.trim() : null), + // Behind the production proxy the direct peer is loopback; the real + // client address comes from the forwarded header the proxy sets — + // see `~/server/request-client` for the trust boundary. + ipAddress: resolveClientIp(event), + host: requestHost(event), // The click carries its own tags when a campaign links straight at the // door; otherwise the landing page's tags ride along in the referer. ...parseUtm(request.url, referrer), @@ -56,6 +96,6 @@ export async function trackDownload( ); await flushAnalytics(); } catch (error) { - console.error(`Failed to log download of ${asset}:`, error); + console.error(`Failed to log ${tracked.method} ${tracked.path}:`, error); } } diff --git a/src/server/weekly-metrics.test.ts b/src/server/weekly-metrics.test.ts index d7321ec..fa42009 100644 --- a/src/server/weekly-metrics.test.ts +++ b/src/server/weekly-metrics.test.ts @@ -1,12 +1,16 @@ import { beforeEach, describe, expect, it } from "bun:test"; import { Database } from "bun:sqlite"; import { + VISITOR_ANALYTICS_ADDED_COLUMNS, VISITOR_ANALYTICS_COLUMNS, VISITOR_ANALYTICS_DDL, VISITOR_ANALYTICS_INSERT } from "./analytics-schema"; import { + NOOK_CHECKOUT_PATH, NOOK_DOWNLOAD_PATH, + NOOK_HOST, + ensureAnalyticsSchema, getWeeklyReport, pickMover, readCampaigns, @@ -70,6 +74,8 @@ let conns: Connections; const NOW = new Date("2026-10-29T16:00:00Z"); +// A row as the post-FRE-366 writers record it: real client address, the host +// it addressed, and a stage label where the row is a download. const row = ( overrides: Record ): Record => ({ @@ -96,6 +102,8 @@ const row = ( ttfb: null, dom_load: null, load_complete: null, + stage: null, + host: NOOK_HOST, ...overrides }); @@ -142,20 +150,32 @@ describe("windowsFor", () => { }); describe("readFunnel", () => { - it("counts each stage inside its window and nothing outside it", async () => { - // Page views count; API calls do not. + it("counts each stage by its own label inside its window", async () => { + // Page views on the Nook host count; other sites, API calls, legacy + // rows with no host, and the other window do not. insertAnalytics({ path: "/", created_at: THIS_WEEK }); insertAnalytics({ path: "/downloads", created_at: THIS_WEEK }); + insertAnalytics({ path: "/", host: "freno.me", created_at: THIS_WEEK }); + insertAnalytics({ path: "/", host: null, created_at: THIS_WEEK }); insertAnalytics({ path: "/api/trpc/anything", created_at: THIS_WEEK }); insertAnalytics({ path: "/", created_at: LAST_WEEK }); - - // The merged download label, click and served alike. - insertAnalytics({ path: NOOK_DOWNLOAD_PATH, created_at: THIS_WEEK }); - insertAnalytics({ path: NOOK_DOWNLOAD_PATH, created_at: THIS_WEEK }); - - // Checkout page doors, including the legacy /nook/checkout path. + // The old checkout proxy: a /checkout page view is a visit, not a + // started checkout. insertAnalytics({ path: "/checkout", created_at: THIS_WEEK }); - insertAnalytics({ path: "/nook/checkout", created_at: THIS_WEEK }); + + // The download label splits by stage; a row with no stage predates the + // split and counts in neither. + insertAnalytics({ path: NOOK_DOWNLOAD_PATH, stage: "click", created_at: THIS_WEEK }); + insertAnalytics({ path: NOOK_DOWNLOAD_PATH, stage: "served", created_at: THIS_WEEK }); + insertAnalytics({ path: NOOK_DOWNLOAD_PATH, stage: "served", created_at: THIS_WEEK }); + insertAnalytics({ path: NOOK_DOWNLOAD_PATH, stage: null, created_at: THIS_WEEK }); + + // A created Stripe session, recorded by the checkout route itself. + insertAnalytics({ + path: NOOK_CHECKOUT_PATH, + method: "POST", + created_at: THIS_WEEK + }); // The licence DB stores ISO-8601 with milliseconds. nookDb @@ -180,13 +200,12 @@ describe("readFunnel", () => { const window = windowsFor(NOW, 7).current; const funnel = await readFunnel(conns, window); - // "/checkout" and "/nook/checkout" are page views too, so they land in - // both the visit count and the checkout count. expect(funnel).toEqual({ - visit: 4, - download: 2, + visit: 3, // "/", "/downloads" and the "/checkout" page view on the Nook host + downloadClick: 1, + dmgServed: 2, trial: 1, - checkout: 2, + checkout: 1, purchase: 1, activation: 1 }); @@ -196,7 +215,8 @@ describe("readFunnel", () => { const window = windowsFor(NOW, 7).current; expect(await readFunnel(conns, window)).toEqual({ visit: 0, - download: 0, + downloadClick: 0, + dmgServed: 0, trial: 0, checkout: 0, purchase: 0, @@ -206,37 +226,51 @@ describe("readFunnel", () => { }); describe("readTraffic", () => { - it("separates loopback and automation from browser traffic", async () => { + it("separates legacy rows, live loopback and automation from clean traffic", async () => { + // Post-instrumentation rows: host set. One of them still loopback — the + // shape the report must flag as "the fix is not live". insertAnalytics({ path: NOOK_DOWNLOAD_PATH, + stage: "click", + created_at: THIS_WEEK + }); + insertAnalytics({ + path: NOOK_DOWNLOAD_PATH, + stage: "served", + ip_address: "127.0.0.1", + user_agent: "The Nook/0.13.4 Sparkle/2.9.6", + created_at: THIS_WEEK + }); + insertAnalytics({ path: "/", created_at: THIS_WEEK }); + + // Pre-instrumentation rows: no host, proxy's address — legacy, never + // counted as "the fix is not live". + insertAnalytics({ + path: NOOK_DOWNLOAD_PATH, + stage: null, + host: null, ip_address: "127.0.0.1", user_agent: "curl/8.7.1", created_at: THIS_WEEK }); insertAnalytics({ - path: NOOK_DOWNLOAD_PATH, + path: "/", + host: null, ip_address: "::1", - user_agent: "The Nook/0.13.4 Sparkle/2.9.6", created_at: THIS_WEEK }); - insertAnalytics({ - path: NOOK_DOWNLOAD_PATH, - ip_address: "203.0.113.9", - user_agent: "Mozilla/5.0 (Macintosh) Safari/605.1.15", - created_at: THIS_WEEK - }); - insertAnalytics({ path: "/", ip_address: "203.0.113.9", created_at: THIS_WEEK }); const traffic = await readTraffic(conns, windowsFor(NOW, 7).current); expect(traffic).toEqual({ - analyticsRows: 4, - loopbackRows: 2, - allTimeRows: 4, - allTimeAddressedRows: 4, - allTimeLoopbackRows: 2, + analyticsRows: 5, + hostRows: 3, + legacyRows: 2, + hostLoopbackRows: 1, downloadRows: 3, - downloadLoopbackRows: 2, + downloadClickRows: 1, + downloadServedRows: 1, + downloadUnstagedRows: 1, downloadNonBrowserRows: 2 }); }); @@ -288,10 +322,53 @@ describe("readCampaigns", () => { }); }); +describe("ensureAnalyticsSchema", () => { + it("adds the post-hoc columns to a table that predates them, idempotently", async () => { + // The table as production held it before any post-hoc column existed. + // (`created_at` is not in the insert column list — it defaults DB-side — + // so the simulated table has to name it explicitly.) + const oldDb = new Database(":memory:"); + const withoutAdded = [ + ...VISITOR_ANALYTICS_COLUMNS.filter( + (c) => !VISITOR_ANALYTICS_ADDED_COLUMNS.some((a) => a.name === c) + ), + "created_at" + ]; + oldDb.run( + `CREATE TABLE VisitorAnalytics (${withoutAdded.map((c) => `${c} TEXT`).join(", ")})` + ); + const oldConn = adapter(oldDb); + + await ensureAnalyticsSchema(oldConn); + await ensureAnalyticsSchema(oldConn); // a second run is a no-op + + // The writers' full INSERT — stage and host included — now works. + const values = row({ stage: "click", path: NOOK_DOWNLOAD_PATH }); + expect(() => + oldDb + .prepare(VISITOR_ANALYTICS_INSERT) + .run( + ...(VISITOR_ANALYTICS_COLUMNS.map((c) => values[c]) as never[]) + ) + ).not.toThrow(); + oldDb + .prepare(`UPDATE VisitorAnalytics SET created_at = ? WHERE id = ?`) + .run(THIS_WEEK, values.id as string); + + // And the funnel reads its stage split from the migrated table. + const funnel = await readFunnel( + { analytics: oldConn, nook: conns.nook }, + windowsFor(NOW, 7).current + ); + expect(funnel.downloadClick).toBe(1); + }); +}); + describe("pickMover", () => { const base: FunnelCounts = { visit: 0, - download: 0, + downloadClick: 0, + dmgServed: 0, trial: 0, checkout: 0, purchase: 0, @@ -300,12 +377,12 @@ describe("pickMover", () => { it("picks the stage with the largest absolute change", () => { const mover = pickMover( - { ...base, visit: 61, download: 156, trial: 2 }, + { ...base, visit: 61, dmgServed: 156, trial: 2 }, { ...base, visit: 56 } ); expect(mover).toEqual({ - key: "download", - label: "download click → DMG served (one label)", + key: "dmgServed", + label: "DMG served", now: 156, before: 0, delta: 156 @@ -314,8 +391,8 @@ describe("pickMover", () => { it("breaks ties toward the earlier stage", () => { const mover = pickMover( - { ...base, visit: 5, download: 5 }, - { ...base, visit: 0, download: 0 } + { ...base, visit: 5, downloadClick: 5 }, + { ...base, visit: 0, downloadClick: 0 } ); expect(mover?.key).toBe("visit"); }); @@ -326,20 +403,20 @@ describe("pickMover", () => { }); describe("getWeeklyReport / renderWeeklyMarkdown", () => { - it("reports a week in the agreed stage order with the computed caveats", async () => { + it("renders seven distinct stages with no merged-label or proxy caveats", async () => { insertAnalytics({ path: "/", created_at: THIS_WEEK }); + insertAnalytics({ path: NOOK_DOWNLOAD_PATH, stage: "click", created_at: THIS_WEEK }); insertAnalytics({ path: NOOK_DOWNLOAD_PATH, - ip_address: "127.0.0.1", - user_agent: "curl/8.7.1", + stage: "served", created_at: THIS_WEEK }); insertAnalytics({ path: NOOK_DOWNLOAD_PATH, - ip_address: "127.0.0.1", - user_agent: "curl/8.7.1", + stage: "served", created_at: THIS_WEEK }); + insertAnalytics({ path: NOOK_CHECKOUT_PATH, method: "POST", created_at: THIS_WEEK }); nookDb .prepare(`INSERT INTO trials (fingerprint, started_at) VALUES (?, ?)`) .run("fp-1", "2026-10-28T20:28:46.806Z"); @@ -347,21 +424,72 @@ describe("getWeeklyReport / renderWeeklyMarkdown", () => { const report = await getWeeklyReport(conns, { now: NOW, days: 7 }); const markdown = renderWeeklyMarkdown(report); - expect(report.mover?.key).toBe("download"); expect(markdown).toContain( "Window: **2026-10-22 16:00:00 → 2026-10-29 16:00:00 UTC** (7 days)" ); - expect(markdown).toContain("| visit (page views) | 1 | 0 | +1 |"); - expect(markdown.indexOf("visit (page views)")).toBeLessThan( - markdown.indexOf("download click → DMG served") + expect(markdown).toContain(`| visit (${NOOK_HOST} page views) | 1 | 0 | +1 |`); + expect(markdown).toContain("| download click | 1 | 0 | +1 |"); + expect(markdown).toContain("| DMG served | 2 | 0 | +2 |"); + expect(markdown).toContain("| checkout started (Stripe session) | 1 | 0 | +1 |"); + + const order = [ + `visit (${NOOK_HOST} page views)`, + "download click", + "DMG served", + "trial started", + "checkout started (Stripe session)", + "purchase (licence issued)", + "activation" + ]; + for (let i = 1; i < order.length; i += 1) { + expect(markdown.indexOf(order[i - 1]!)).toBeLessThan( + markdown.indexOf(order[i]!) + ); + } + + // The caveats this issue retired stay retired. + expect(markdown).not.toContain("one label"); + expect(markdown).not.toContain("not recorded"); + expect(markdown).not.toContain("no host dimension"); + expect(markdown).not.toContain("†"); + + expect(report.mover?.key).toBe("dmgServed"); + expect(markdown).toContain("**DMG served: 0 → 2 (+2).**"); + }); + + it("states what pre-instrumentation rows cannot say, computed from the window", async () => { + insertAnalytics({ path: "/", created_at: THIS_WEEK }); + // Legacy rows: no host, proxy's address, no stage label. + insertAnalytics({ path: "/", host: null, ip_address: "127.0.0.1", created_at: THIS_WEEK }); + insertAnalytics({ + path: NOOK_DOWNLOAD_PATH, + stage: null, + host: null, + ip_address: "127.0.0.1", + user_agent: "curl/8.7.1", + created_at: THIS_WEEK + }); + + const report = await getWeeklyReport(conns, { now: NOW, days: 7 }); + const markdown = renderWeeklyMarkdown(report); + + expect(markdown).toContain( + "2 of 3 rows in the window predate the FRE-366 instrumentation" ); - expect(markdown.indexOf("download click → DMG served")).toBeLessThan( - markdown.indexOf("trial started") + expect(markdown).toContain("predate the click/served stage label"); + // Legacy loopback is not "the fix is not live". + expect(markdown).not.toContain("still carry a loopback address"); + }); + + it("flags loopback only when rows recorded since the instrumentation carry it", async () => { + insertAnalytics({ path: "/", ip_address: "127.0.0.1", created_at: THIS_WEEK }); + + const report = await getWeeklyReport(conns, { now: NOW, days: 7 }); + const markdown = renderWeeklyMarkdown(report); + + expect(markdown).toContain( + "1 rows recorded since the instrumentation still carry a loopback address" ); - expect(markdown).toContain("**download click → DMG served (one label): 0 → 2 (+2).**"); - expect(markdown).toContain("2 of 3 rows in the window came from a loopback address"); - expect(markdown).toContain("Checkout start is not recorded"); - expect(markdown).toContain("There is no host dimension"); }); it("says nothing moved rather than inventing a winner", async () => { diff --git a/src/server/weekly-metrics.ts b/src/server/weekly-metrics.ts index 7ab1b72..25f1130 100644 --- a/src/server/weekly-metrics.ts +++ b/src/server/weekly-metrics.ts @@ -18,20 +18,33 @@ * whose `.env` carries nothing else. The SQL here is the same shape as the * analytics summaries, so the weekly read and the dashboard cannot drift * into different label sets. - * - Every stage carries what it actually measured. Two of the seven agreed - * stages share one label in production today and a third is only a page-view - * proxy; the report states that next to the number instead of inventing a - * figure (see `renderWeeklyMarkdown`). + * - Every stage is measured by its own label. FRE-366 gave the funnel that: + * the download door and the DMG file route record distinct `stage` values + * (`click` / `served`), a created Stripe checkout session records its own + * row, and every row carries the real client address and the host it + * addressed, so the visit stage scopes to the Nook. Rows that predate + * that instrumentation are stated next to the numbers, never silently + * counted (see `renderWeeklyMarkdown`). * * No `~/env`, no `~/server/database`: the caller owns the connections, the * tests own an in-memory SQLite, and the script owns the libsql clients. */ import { DOWNLOAD_PATH_PREFIX } from "~/lib/download-asset"; +import { + VISITOR_ANALYTICS_ADDED_COLUMNS, + VISITOR_ANALYTICS_DDL +} from "~/server/analytics-schema"; /** The one asset key the Nook lands under — `downloadPath("thenook")`. */ export const NOOK_DOWNLOAD_PATH = `${DOWNLOAD_PATH_PREFIX}thenook`; +/** The host the visit stage scopes to. Recorded per row since FRE-366. */ +export const NOOK_HOST = "nook.freno.me"; + +/** The path a created Stripe checkout session records (FRE-366). */ +export const NOOK_CHECKOUT_PATH = "/api/the-nook/checkout"; + /** An `execute`-shaped connection: the libsql client and the test adapter both fit. */ export interface MetricsConn { execute(query: { @@ -49,7 +62,8 @@ export interface Connections { export type StageKey = | "visit" - | "download" + | "downloadClick" + | "dmgServed" | "trial" | "checkout" | "purchase" @@ -58,35 +72,28 @@ export type StageKey = export interface StageDef { key: StageKey; label: string; - /** True when the number is a proxy or a merged label, not a clean stage. */ - approximate: boolean; } /** - * The seven agreed stages. `download` deliberately carries two of them: the - * door and the file route both write `/api/downloads/thenook`, so a click and - * the DMG it fetched are one label in the data (proved in the report's caveats). + * The seven agreed stages, each with its own label in the data: + * `downloadClick` and `dmgServed` split what used to be one merged download + * label (the `stage` column, FRE-366), and `checkout` reads created Stripe + * sessions instead of the /checkout page-view proxy. */ export const STAGES: StageDef[] = [ - { key: "visit", label: "visit (page views)", approximate: false }, - { - key: "download", - label: "download click → DMG served (one label)", - approximate: true - }, - { key: "trial", label: "trial started", approximate: false }, - { - key: "checkout", - label: "checkout page visit", - approximate: true - }, - { key: "purchase", label: "purchase (licence issued)", approximate: false }, - { key: "activation", label: "activation", approximate: false } + { key: "visit", label: `visit (${NOOK_HOST} page views)` }, + { key: "downloadClick", label: "download click" }, + { key: "dmgServed", label: "DMG served" }, + { key: "trial", label: "trial started" }, + { key: "checkout", label: "checkout started (Stripe session)" }, + { key: "purchase", label: "purchase (licence issued)" }, + { key: "activation", label: "activation" } ]; export interface FunnelCounts { visit: number; - download: number; + downloadClick: number; + dmgServed: number; trial: number; checkout: number; purchase: number; @@ -103,13 +110,17 @@ export interface CampaignRow { /** Traffic-shape facts the report has to state next to the funnel. */ export interface WindowTraffic { analyticsRows: number; - loopbackRows: number; - /** All-time row counts, so "every row ever" is a checked claim, not a guess. */ - allTimeRows: number; - allTimeAddressedRows: number; - allTimeLoopbackRows: number; + /** Rows carrying a host — written since the FRE-366 instrumentation. */ + hostRows: number; + /** Rows with no host — pre-FRE-366; their address is the proxy's loopback. */ + legacyRows: number; + /** Post-instrumentation rows still recording loopback: the fix is not live. */ + hostLoopbackRows: number; downloadRows: number; - downloadLoopbackRows: number; + downloadClickRows: number; + downloadServedRows: number; + /** Download rows with no stage label — they predate FRE-366 and cannot be split. */ + downloadUnstagedRows: number; downloadNonBrowserRows: number; } @@ -183,6 +194,28 @@ async function scalar( return toNumber(Object.values(row)[0]); } +/** + * Bring the analytics table up to the schema the readers expect, through the + * same idempotent bootstrap path the app server uses (`analytics.ts`): the + * DDL is a no-op against the live table and each post-hoc column is ALTERed + * in only when absent. The weekly job runs against databases the app may not + * have migrated yet — a fresh checkout, a restored backup, or production + * before the next deploy — and must render its seven stages regardless. + */ +export async function ensureAnalyticsSchema(conn: MetricsConn): Promise { + await conn.execute({ sql: VISITOR_ANALYTICS_DDL }); + + const info = await conn.execute({ + sql: `PRAGMA table_info(VisitorAnalytics)` + }); + const present = new Set(info.rows.map((row) => String((row as any).name))); + for (const column of VISITOR_ANALYTICS_ADDED_COLUMNS) { + if (!present.has(column.name)) { + await conn.execute({ sql: column.ddl }); + } + } +} + async function countInWindow( conn: MetricsConn, sql: string, @@ -200,30 +233,43 @@ export async function readFunnel( const { analytics, nook } = conns; const { start, end } = window; - const visit = await countInWindow( + // Scoped to the Nook host (FRE-366): before the host column existed, a + // page view of `/` on nook.freno.me was the same row as freno.me's root. + // Pre-instrumentation rows carry no host and are excluded — the traffic + // section states how many. + const visit = await scalar( analytics, `SELECT COUNT(*) AS n FROM VisitorAnalytics WHERE created_at >= ? AND created_at < ? - AND path NOT LIKE '/api/%'`, - start, - end + AND path NOT LIKE '/api/%' + AND host = ?`, + [start, end, NOOK_HOST] ); - const download = await scalar( + const downloadClick = await scalar( + analytics, + `SELECT COUNT(*) AS n FROM VisitorAnalytics + WHERE created_at >= ? AND created_at < ? + AND path = ? AND stage = 'click'`, + [start, end, NOOK_DOWNLOAD_PATH] + ); + + const dmgServed = await scalar( + analytics, + `SELECT COUNT(*) AS n FROM VisitorAnalytics + WHERE created_at >= ? AND created_at < ? + AND path = ? AND stage = 'served'`, + [start, end, NOOK_DOWNLOAD_PATH] + ); + + // Created Stripe checkout sessions, recorded by the checkout route itself + // (FRE-366) — not the /checkout page view that used to proxy for them. + const checkout = await scalar( analytics, `SELECT COUNT(*) AS n FROM VisitorAnalytics WHERE created_at >= ? AND created_at < ? AND path = ?`, - [start, end, NOOK_DOWNLOAD_PATH] - ); - - const checkout = await countInWindow( - analytics, - `SELECT COUNT(*) AS n FROM VisitorAnalytics - WHERE created_at >= ? AND created_at < ? - AND (path LIKE '/checkout%' OR path LIKE '/nook/checkout%')`, - start, - end + [start, end, NOOK_CHECKOUT_PATH] ); // The licence DB stores ISO-8601 (`2026-10-28T20:28:46.806Z`); the analytics @@ -252,7 +298,8 @@ export async function readFunnel( return { visit, - download, + downloadClick, + dmgServed, trial, checkout, purchase, @@ -261,9 +308,11 @@ export async function readFunnel( } /** - * Traffic shape for one window: how much of it came from this host, and how - * much of the download label was automation. Without these two facts the - * numbers read as demand, which they are not. + * Traffic shape for one window: how much of it predates the FRE-366 + * instrumentation (no host, proxy's address), whether rows recorded since + * still see loopback (which would mean the forwarded-header fix is not + * live), and how the download label splits. Without these facts the numbers + * read as demand, which they are not. */ export async function readTraffic( conns: Connections, @@ -273,25 +322,6 @@ export async function readTraffic( const { start, end } = window; const placeholders = LOOPBACK.map(() => "?").join(", "); - const allTimeRows = await scalar( - analytics, - `SELECT COUNT(*) AS n FROM VisitorAnalytics`, - [] - ); - - const allTimeAddressedRows = await scalar( - analytics, - `SELECT COUNT(*) AS n FROM VisitorAnalytics WHERE ip_address IS NOT NULL`, - [] - ); - - const allTimeLoopbackRows = await scalar( - analytics, - `SELECT COUNT(*) AS n FROM VisitorAnalytics - WHERE ip_address IN (${placeholders})`, - [...LOOPBACK] - ); - const analyticsRows = await countInWindow( analytics, `SELECT COUNT(*) AS n FROM VisitorAnalytics @@ -300,10 +330,27 @@ export async function readTraffic( end ); - const loopbackRows = await scalar( + const hostRows = await scalar( analytics, `SELECT COUNT(*) AS n FROM VisitorAnalytics WHERE created_at >= ? AND created_at < ? + AND host IS NOT NULL`, + [start, end] + ); + + const legacyRows = await scalar( + analytics, + `SELECT COUNT(*) AS n FROM VisitorAnalytics + WHERE created_at >= ? AND created_at < ? + AND host IS NULL`, + [start, end] + ); + + const hostLoopbackRows = await scalar( + analytics, + `SELECT COUNT(*) AS n FROM VisitorAnalytics + WHERE created_at >= ? AND created_at < ? + AND host IS NOT NULL AND ip_address IN (${placeholders})`, [start, end, ...LOOPBACK] ); @@ -315,12 +362,25 @@ export async function readTraffic( [start, end, NOOK_DOWNLOAD_PATH] ); - const downloadLoopbackRows = await scalar( + const downloadClickRows = await scalar( analytics, `SELECT COUNT(*) AS n FROM VisitorAnalytics - WHERE created_at >= ? AND created_at < ? AND path = ? - AND ip_address IN (${placeholders})`, - [start, end, NOOK_DOWNLOAD_PATH, ...LOOPBACK] + WHERE created_at >= ? AND created_at < ? AND path = ? AND stage = 'click'`, + [start, end, NOOK_DOWNLOAD_PATH] + ); + + const downloadServedRows = await scalar( + analytics, + `SELECT COUNT(*) AS n FROM VisitorAnalytics + WHERE created_at >= ? AND created_at < ? AND path = ? AND stage = 'served'`, + [start, end, NOOK_DOWNLOAD_PATH] + ); + + const downloadUnstagedRows = await scalar( + analytics, + `SELECT COUNT(*) AS n FROM VisitorAnalytics + WHERE created_at >= ? AND created_at < ? AND path = ? AND stage IS NULL`, + [start, end, NOOK_DOWNLOAD_PATH] ); // Anything whose UA is not a browser engine: curl, Sparkle's updater, @@ -335,12 +395,13 @@ export async function readTraffic( return { analyticsRows, - loopbackRows, - allTimeRows, - allTimeAddressedRows, - allTimeLoopbackRows, + hostRows, + legacyRows, + hostLoopbackRows, downloadRows, - downloadLoopbackRows, + downloadClickRows, + downloadServedRows, + downloadUnstagedRows, downloadNonBrowserRows }; } @@ -483,8 +544,7 @@ export function renderWeeklyMarkdown(report: WeeklyReport): string { for (const stage of STAGES) { const now = counts[stage.key]; const before = previousCounts[stage.key]; - const label = stage.approximate ? `${stage.label} †` : stage.label; - lines.push(`| ${label} | ${now} | ${before} | ${signed(now - before)} |`); + lines.push(`| ${stage.label} | ${now} | ${before} | ${signed(now - before)} |`); } lines.push(""); @@ -528,58 +588,45 @@ export function renderWeeklyMarkdown(report: WeeklyReport): string { lines.push("## Before acting on these numbers"); lines.push(""); - if (traffic.analyticsRows > 0 && traffic.loopbackRows > 0) { + if (traffic.legacyRows > 0) { lines.push( - `- **${traffic.loopbackRows} of ${traffic.analyticsRows} rows in the window came from a loopback address** ` + - `(127.0.0.1 / ::1)` + - (traffic.allTimeAddressedRows > 0 && - traffic.allTimeAddressedRows === traffic.allTimeLoopbackRows - ? `; every one of the ${traffic.allTimeAddressedRows} addressed rows the table ` + - `has ever held is loopback (${traffic.allTimeRows - traffic.allTimeAddressedRows} carry no address)` - : `; across the table's history, ${traffic.allTimeLoopbackRows} of ` + - `${traffic.allTimeAddressedRows} addressed rows are loopback`) + - `. Either this host is the only audience, or the app sits behind a local proxy ` + - `and \`getRequestIP\` records the proxy, not the visitor. Either way the address ` + - `identifies nobody — "unique visitors" collapses to one host and per-channel ` + - `reach cannot be read from it.` + `- **${traffic.legacyRows} of ${traffic.analyticsRows} rows in the window predate the FRE-366 instrumentation.** ` + + `They carry no host, and their recorded address is the production proxy's loopback ` + + `(127.0.0.1 / ::1) — the writers then recorded the direct peer, not the visitor. ` + + `They are excluded from every host-scoped stage above (visits, and the download ` + + `click/served split), and "unique visitors" over them still collapses to one host. ` + + `Rows recorded since carry the real client address and the site they addressed.` + ); + } + + if (traffic.hostLoopbackRows > 0) { + lines.push( + `- **${traffic.hostLoopbackRows} rows recorded since the instrumentation still carry a ` + + `loopback address** — the forwarded-header fix is not taking effect in production ` + + `(or the proxy is not setting \`x-forwarded-for\`). Do not trust per-channel reach ` + + `until this is zero.` ); } if (traffic.downloadRows > 0) { lines.push( - `- **The download stage is one label, not two.** The landing-door route and the file ` + - `route that serves the DMG both write \`${NOOK_DOWNLOAD_PATH}\`, so ` + - `"download click → DMG served" cannot be split: one served click writes two rows, ` + - `a click that fails to resolve writes one, and a Sparkle update writes one. ` + - `Of this window's ${traffic.downloadRows} download rows, ` + - `${traffic.downloadLoopbackRows} are loopback and ` + - `${traffic.downloadNonBrowserRows} are not browser user agents (curl, Sparkle, ` + - `link-preview crawlers).` + `- **Download rows are automation-heavy.** Of this window's ${traffic.downloadRows} ` + + `download rows, ${traffic.downloadClickRows} are clicks and ` + + `${traffic.downloadServedRows} are served files` + + (traffic.downloadUnstagedRows > 0 + ? `; the remaining ${traffic.downloadUnstagedRows} predate the click/served stage ` + + `label and cannot be split, so they count in neither stage` + : "") + + `. ${traffic.downloadNonBrowserRows} of all ${traffic.downloadRows} are not browser ` + + `user agents (curl, Sparkle, link-preview crawlers).` ); } - lines.push( - `- **Checkout start is not recorded.** \`POST /api/the-nook/checkout\` — the call that ` + - `actually creates the Stripe session — writes nothing to analytics; the only trace is ` + - `the /checkout page view reported above, which is a page, not a started checkout.` - ); - - lines.push( - `- **There is no host dimension.** \`VisitorAnalytics\` records a path, not the site, so ` + - `the Nook's landing page (\`/\` on nook.freno.me) is the same row as freno.me's root. ` + - `"visit" above is site-wide page views, not Nook visits.` - ); - lines.push( `- **No conversion rate before the cohort matures.** Trial→paid belongs to the mature ` + `Day 28–35 read (FRE-197); a ratio from a partial cohort is not a metric.` ); lines.push(""); - - lines.push( - "† Proxy or merged label — see *Before acting on these numbers* above." - ); - lines.push(""); lines.push("## Sources"); lines.push(""); lines.push("- `VisitorAnalytics` (freno.me Turso DB) — visits, downloads, campaigns.");