From c4a6b0615e1fe7ab5c5a1db9d18aea1a357c9f03 Mon Sep 17 00:00:00 2001 From: Brian Love Date: Fri, 18 Sep 2026 11:38:10 -0700 Subject: [PATCH] fix(posthog): the weekly report reports real numbers Every row rendered Unavailable for two independent reasons: the insight fetch asked PostHog for `refresh: 'force_cache'`, which never recomputes, so a stale or short cache failed the complete-28-day-window check; and any insight carrying a breakdown was refused outright, which covers 9 of the 13 managed insights. The fetch now asks for `refresh: 'blocking'`, which serves a fresh cache and otherwise recalculates synchronously. If PostHog still answers with a pending query_status, we re-read the cache a bounded number of times and leave the row Unavailable rather than hanging or printing a zero. Breakdown insights are no longer refused. Because breakdown_limit means PostHog returns only the top N values, summing them would undercount, so we re-issue the insight's own trends query with the breakdown stripped and label the row "total across all breakdown values". Every existing refusal that protects correctness is unchanged: non-trends insights, non-daily intervals, non-additive math, and short or undated result series still render Unavailable with their current reasons. Co-Authored-By: Claude Opus 5 --- tools/posthog/report.spec.ts | 85 +++++++++++++++++++++++++++++++++++- tools/posthog/report.ts | 62 +++++++++++++++++++++++--- 2 files changed, 141 insertions(+), 6 deletions(-) diff --git a/tools/posthog/report.spec.ts b/tools/posthog/report.spec.ts index 51f1ff562..8f97567a5 100644 --- a/tools/posthog/report.spec.ts +++ b/tools/posthog/report.spec.ts @@ -1,6 +1,6 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { sparkline, formatDeltaCell, renderReport, insightReportRows, generateReport } from './report.js'; +import { sparkline, formatDeltaCell, renderReport, insightReportRows, generateReport, BREAKDOWN_TOTAL_SUFFIX } from './report.js'; const asOf = new Date('2026-09-07T17:45:00Z'); const dayMs = 86_400_000; @@ -153,3 +153,86 @@ test('renderReport: produces stable markdown structure', () => { assert(out.includes('Last 7 complete UTC days')); assert(out.includes('Preceding 7 UTC days')); }); + +const breakdownSource = { kind: 'TrendsQuery', interval: 'day', series: [{ math: 'total' }], breakdownFilter: { breakdown_type: 'event', breakdown: 'cta_id', breakdown_limit: 15 } }; + +function dashboardClient(insight: any, extra: Record = {}) { + return { async GET(path: string, options: any) { + if (path === '/dashboards/') return { data: { results: [{ id: 10, name: 'Managed', tags: ['gtm'] }], next: null } }; + if (path === '/dashboards/{id}/') return { data: { id: 10, name: 'Managed', tiles: [{ insight: { id: 7 } }] } }; + assert.equal(path, '/insights/{id}/'); + assert.equal(options.params.query.refresh, 'blocking'); + return { data: insight }; + }, ...extra }; +} + +test('report fetches insights with a refresh that recomputes a stale cache', async () => { + const refreshes: string[] = []; + const client = { async GET(path: string, options: any) { + if (path === '/dashboards/') return { data: { results: [{ id: 10, name: 'Managed' }], next: null } }; + if (path === '/dashboards/{id}/') return { data: { id: 10, name: 'Managed', tiles: [{ insight: 7 }] } }; + refreshes.push(options.params.query.refresh); + return { data: { id: 7, name: 'Accepted', query: { source: dailySource }, result: [{ days, data: Array(28).fill(1) }] } }; + } }; + const report = await generateReport({ client, asOf, dashboardIds: [10] }); + assert.deepEqual(refreshes, ['blocking']); + assert.match(report.markdown, /\| Accepted \| 7 \| 7/); +}); + +test('report totals a breakdown insight by re-running the query without the breakdown', async () => { + const bodies: any[] = []; + const client = dashboardClient( + { id: 7, name: 'Install clicks', query: { source: breakdownSource }, result: [{ days, data: Array(28).fill(1), label: 'hero_install' }] }, + { async POST(path: string, options: any) { + assert.equal(path, '/query/'); + bodies.push(options.body); + return { data: { results: [{ days, data: Array(28).fill(3) }] } }; + } }, + ); + const report = await generateReport({ client, asOf, dashboardIds: [10] }); + assert.equal(bodies.length, 1); + assert.equal(bodies[0].refresh, 'blocking'); + assert.equal(bodies[0].query.kind, 'TrendsQuery'); + assert.equal(bodies[0].query.breakdownFilter, undefined); + assert.match(report.markdown, new RegExp(`Install clicks${BREAKDOWN_TOTAL_SUFFIX} \\| 21 \\| 21`)); + assert.doesNotMatch(report.markdown, /Unavailable/); +}); + +test('report still refuses non-daily, non-trends and non-additive insights', () => { + const cases: Array<[any, RegExp]> = [ + [{ id: 1, name: 'Weekly', query: { source: { ...dailySource, interval: 'week' } }, result: [{ days, data: Array(28).fill(1) }] }, /daily/i], + [{ id: 2, name: 'Funnel', query: { source: { kind: 'FunnelsQuery' } }, result: [] }, /unsupported/i], + [{ id: 3, name: 'Uniques', query: { source: { ...dailySource, series: [{ math: 'dau' }] } }, result: [{ days, data: Array(28).fill(1) }] }, /unique/i], + ]; + for (const [insight, reason] of cases) { + const rows = insightReportRows(insight, asOf); + assert.equal(rows[0].thisWeek, null); + assert.match(rows[0].unavailable ?? '', reason); + } +}); + +test('report refuses a breakdown total whose recomputed series is short or missing', async () => { + for (const data of [undefined, { results: [{ days: days.slice(0, 10), data: Array(10).fill(1) }] }]) { + const client = dashboardClient( + { id: 7, name: 'Install clicks', query: { source: breakdownSource }, result: [] }, + { async POST() { return { data: data ?? { results: [] } }; } }, + ); + const report = await generateReport({ client, asOf, dashboardIds: [10] }); + assert.match(report.markdown, /Unavailable/); + } +}); + +test('report polls a bounded number of times when PostHog answers with a pending query status', async () => { + const waits: number[] = []; + let calls = 0; + const client = { async GET(path: string) { + if (path === '/dashboards/') return { data: { results: [{ id: 10, name: 'Managed' }], next: null } }; + if (path === '/dashboards/{id}/') return { data: { id: 10, name: 'Managed', tiles: [{ insight: 7 }] } }; + calls += 1; + return { data: { id: 7, name: 'Pending', query: { source: dailySource }, query_status: { complete: false } } }; + } }; + const report = await generateReport({ client, asOf, dashboardIds: [10], sleep: async (ms: number) => { waits.push(ms); } }); + assert.equal(calls, 11); + assert.equal(waits.length, 10); + assert.match(report.markdown, /Pending \| Unavailable/); +}); diff --git a/tools/posthog/report.ts b/tools/posthog/report.ts index a88614d06..3e12759bb 100644 --- a/tools/posthog/report.ts +++ b/tools/posthog/report.ts @@ -131,6 +131,55 @@ function expectOk(r: { data?: T; error?: unknown }, op: string): T { interface ReportClient { GET(path: string, options?: any): Promise<{ data?: unknown; error?: unknown }>; + POST?(path: string, options?: any): Promise<{ data?: unknown; error?: unknown }>; +} + +/** + * Breakdown insights carry a `breakdown_limit`, so PostHog returns only the top N + * values and summing the returned series would undercount. We re-run the same + * trends query with the breakdown removed and label the row accordingly. + */ +export const BREAKDOWN_TOTAL_SUFFIX = ' — total across all breakdown values'; + +/** `blocking` recomputes a stale cache and returns only when the query is done. */ +const REFRESH = 'blocking' as const; +const POLL_ATTEMPTS = 10; +const POLL_INTERVAL_MS = 3_000; + +function trendsSource(insight: FetchedInsight): any { + return insight.query?.kind === 'TrendsQuery' ? insight.query : insight.query?.source; +} + +function stillComputing(insight: FetchedInsight | undefined): boolean { + const status = (insight as any)?.query_status; + return !Array.isArray(insight?.result) && !!status && status.complete !== true; +} + +/** + * `blocking` normally returns a computed result, but PostHog may hand back a + * query_status instead. Poll the cache a bounded number of times rather than + * hanging — an insight that never completes stays Unavailable, never zero. + */ +async function fetchInsight(c: ReportClient, id: number, sleep: (ms: number) => Promise): Promise { + const get = async (refresh: string) => + expectOk(await c.GET('/insights/{id}/' as any, { params: { path: { id }, query: { refresh } } } as any) as any, `get insight ${id}`) as FetchedInsight; + let insight = await get(REFRESH); + for (let attempt = 0; attempt < POLL_ATTEMPTS && stillComputing(insight); attempt += 1) { + await sleep(POLL_INTERVAL_MS); + insight = await get('force_cache'); + } + return insight; +} + +/** Re-runs the insight's own trends query with the breakdown stripped. */ +async function breakdownTotalRows(c: ReportClient, insight: FetchedInsight, source: any, asOf: Date): Promise { + if (typeof c.POST !== 'function') return insightReportRows(insight, asOf); + const flat = { ...source }; + delete flat.breakdownFilter; + const response = await c.POST('/query/' as any, { body: { query: flat, refresh: REFRESH } } as any); + const payload = expectOk(response as any, `query insight ${insight.id} without breakdown`) as any; + const result = Array.isArray(payload?.results) ? payload.results : payload?.result; + return insightReportRows({ ...insight, name: `${insight.name}${BREAKDOWN_TOTAL_SUFFIX}`, query: flat, result }, asOf); } async function managedDashboardIds(): Promise { @@ -145,10 +194,11 @@ async function managedDashboardIds(): Promise { return ids; } -export async function generateReport(options: { client?: ReportClient; asOf?: Date; dashboardIds?: readonly number[] } = {}): Promise<{ markdown: string; date: string }> { +export async function generateReport(options: { client?: ReportClient; asOf?: Date; dashboardIds?: readonly number[]; sleep?: (ms: number) => Promise } = {}): Promise<{ markdown: string; date: string }> { const asOf = options.asOf ?? new Date(); const c = options.client ?? ph(); const ids = options.dashboardIds ?? await managedDashboardIds(); + const sleep = options.sleep ?? ((ms: number) => new Promise(resolve => setTimeout(resolve, ms))); const dashboards = await fetchAllPages(async (offset, limit) => { const response = await c.GET('/dashboards/' as any, { params: { query: { limit, offset } } } as any); return expectOk(response, 'list dashboards') as { results: FetchedDashboard[]; next?: string | null }; @@ -164,10 +214,12 @@ export async function generateReport(options: { client?: ReportClient; asOf?: Da for (const tile of d.tiles) { const tileId = typeof tile.insight === 'number' ? tile.insight : tile.insight?.id; if (typeof tileId !== 'number') continue; - const insightRes = await c.GET(`/insights/{id}/` as any, { - params: { path: { id: tileId }, query: { refresh: 'force_cache' } }, - } as any); - const insight = expectOk(insightRes as any, `get insight ${tileId}`) as FetchedInsight; + const insight = await fetchInsight(c, tileId, sleep); + const source = trendsSource(insight); + if (source?.kind === 'TrendsQuery' && source.breakdownFilter?.breakdown) { + rows.push(...await breakdownTotalRows(c, insight, source, asOf)); + continue; + } rows.push(...insightReportRows(insight, asOf)); } if (!rows.length) rows.push({ metric: d.name, thisWeek: null, lastWeek: null, weeks: [], unavailable: 'no insight tiles on this dashboard' });