diff --git a/packages/api-client/src/posthog-client.test.ts b/packages/api-client/src/posthog-client.test.ts index 7486815ad3..4a03d1fecb 100644 --- a/packages/api-client/src/posthog-client.test.ts +++ b/packages/api-client/src/posthog-client.test.ts @@ -1147,6 +1147,136 @@ describe("PostHogAPIClient", () => { await expect(client.getSignalReport("abc")).resolves.toEqual({ id: "abc", title: "hi", + charts: [], + }); + }); + + // A chart's `query` is POSTed back to `/api/projects/:id/query/` to draw it, + // so normalization is the gate on what the app is willing to execute. + describe("charts", () => { + const trendsQuery = { + kind: "InsightVizNode", + source: { kind: "TrendsQuery" }, + }; + + async function fetchCharts(charts: unknown) { + const fetch = vi.fn().mockResolvedValue({ + json: async () => ({ id: "abc", title: "hi", charts }), + }); + const report = await makeClient(fetch).getSignalReport("abc"); + return report?.charts; + } + + it("keeps a well-formed chart", async () => { + await expect( + fetchCharts([ + { + chart_id: "signups", + title: "Daily signups", + query: trendsQuery, + caption: "Since Friday", + size: "large", + }, + ]), + ).resolves.toEqual([ + { + chart_id: "signups", + title: "Daily signups", + query: trendsQuery, + caption: "Since Friday", + size: "large", + }, + ]); + }); + + it("normalizes an absent caption and size to null", async () => { + await expect( + fetchCharts([{ chart_id: "a", title: "A", query: trendsQuery }]), + ).resolves.toEqual([ + { + chart_id: "a", + title: "A", + query: trendsQuery, + caption: null, + size: null, + }, + ]); + }); + + it.each([ + { name: "a missing id", chart: { title: "A", query: trendsQuery } }, + { + name: "a malformed id", + chart: { chart_id: "Not Valid", title: "A", query: trendsQuery }, + }, + { + name: "a blank title", + chart: { chart_id: "a", title: " ", query: trendsQuery }, + }, + { + name: "a missing query", + chart: { chart_id: "a", title: "A" }, + }, + { + name: "an unrenderable query kind", + chart: { chart_id: "a", title: "A", query: { kind: "TrendsQuery" } }, + }, + { + name: "an executable query nested in the node", + chart: { + chart_id: "a", + title: "A", + query: { kind: "InsightVizNode", source: { kind: "HogQuery" } }, + }, + }, + { + name: "bytecode smuggled into the node", + chart: { + chart_id: "a", + title: "A", + query: { kind: "InsightVizNode", source: { bytecode: ["_H", 1] } }, + }, + }, + ])("drops a chart with $name", async ({ chart }) => { + await expect(fetchCharts([chart])).resolves.toEqual([]); + }); + + it("keeps the good charts when one entry is malformed", async () => { + const charts = await fetchCharts([ + { chart_id: "bad" }, + { chart_id: "good", title: "Good", query: trendsQuery }, + ]); + + expect(charts?.map((chart) => chart.chart_id)).toEqual(["good"]); + }); + + it("collapses a duplicate id to its first occurrence", async () => { + const charts = await fetchCharts([ + { chart_id: "a", title: "First", query: trendsQuery }, + { chart_id: "a", title: "Second", query: trendsQuery }, + ]); + + expect(charts).toHaveLength(1); + expect(charts?.[0].title).toBe("First"); + }); + + it("caps the chart list at the backend's limit", async () => { + const charts = await fetchCharts( + Array.from({ length: 25 }, (_entry, index) => ({ + chart_id: `c${index}`, + title: `Chart ${index}`, + query: trendsQuery, + })), + ); + + expect(charts).toHaveLength(20); + }); + + it.each([ + { name: "absent", charts: undefined }, + { name: "not an array", charts: "nope" }, + ])("yields no charts when the field is $name", async ({ charts }) => { + await expect(fetchCharts(charts)).resolves.toEqual([]); }); }); diff --git a/packages/api-client/src/posthog-client.ts b/packages/api-client/src/posthog-client.ts index 9a42697e08..24feb49269 100644 --- a/packages/api-client/src/posthog-client.ts +++ b/packages/api-client/src/posthog-client.ts @@ -7,6 +7,7 @@ import type { CreateTaskAutomationOptions, ExecutionMode, PrAuthorshipMode, + ReportChart, SourceProduct, SourceType, StoredLogEntry, @@ -21,7 +22,14 @@ import { DISMISSAL_REASON_OPTIONS, type DismissalReasonOptionValue, getCloudTaskGatewayUrl, + hasForbiddenReportChartQueryNode, + isReportChartId, + isReportChartQueryKind, + isReportChartSize, isSupportedReasoningEffort, + MAX_REPORT_CHART_CAPTION_LENGTH, + MAX_REPORT_CHART_TITLE_LENGTH, + MAX_REPORT_CHARTS, normalizeGatewayModelsResponse, resolveCloudInitialPermissionMode, taskAutomationListSchema, @@ -1397,6 +1405,71 @@ function parseSignalReportArtefactsPayload( }; } +// ── Report charts ───────────────────────────────────────────────────────── +// A chart's `query` is POSTed back to `/api/projects/:id/query/` to draw it, so +// this is a trust boundary rather than a cosmetic parse: a chart missing its id, +// carrying a query kind we don't render, or nesting an executable node is +// dropped here and never reaches `runInsightQuery`. Malformed entries drop +// individually — one bad chart shouldn't cost the report its other evidence. + +function normalizeReportChart(value: unknown): ReportChart | null { + if (!isObjectRecord(value)) return null; + + const chartId = optionalString(value.chart_id); + if (!chartId || !isReportChartId(chartId)) return null; + + const title = optionalString(value.title)?.trim(); + if (!title || title.length > MAX_REPORT_CHART_TITLE_LENGTH) return null; + + if (!isObjectRecord(value.query) || Array.isArray(value.query)) return null; + const query = value.query as Record; + if (!isReportChartQueryKind(query.kind)) return null; + if (hasForbiddenReportChartQueryNode(query)) return null; + + const caption = optionalString(value.caption); + return { + chart_id: chartId, + title, + query, + caption: + caption && caption.length <= MAX_REPORT_CHART_CAPTION_LENGTH + ? caption + : null, + size: isReportChartSize(value.size) ? value.size : null, + }; +} + +/** + * The renderable charts on a report payload. Duplicate ids collapse to the first + * occurrence, since the summary places charts by id and a repeat would be + * unreachable anyway. + */ +function normalizeReportCharts(value: unknown): ReportChart[] { + if (!Array.isArray(value)) return []; + const charts: ReportChart[] = []; + const seen = new Set(); + for (const entry of value) { + if (charts.length >= MAX_REPORT_CHARTS) break; + const chart = normalizeReportChart(entry); + if (!chart || seen.has(chart.chart_id)) continue; + seen.add(chart.chart_id); + charts.push(chart); + } + return charts; +} + +/** + * Report rows arrive as raw JSON casts; this replaces just the `charts` field + * with its validated form, leaving every other field as the backend sent it. + */ +function withNormalizedReportCharts(report: T): T { + if (!isObjectRecord(report)) return report; + return { + ...report, + charts: normalizeReportCharts((report as Record).charts), + } as T; +} + function normalizeAvailableSuggestedReviewer( uuid: string, value: unknown, @@ -3985,7 +4058,9 @@ export class PostHogAPIClient { url, path, }); - return (await response.json()) as SignalReport; + return withNormalizedReportCharts( + (await response.json()) as SignalReport, + ); } catch (error) { // The shared fetcher throws "Failed request: [] " for any // non-2xx. Treat missing / forbidden as "not available in the current @@ -4049,7 +4124,7 @@ export class PostHogAPIClient { count?: number; }; return { - results: data.results ?? [], + results: (data.results ?? []).map(withNormalizedReportCharts), count: data.count ?? data.results?.length ?? 0, }; } @@ -4135,7 +4210,7 @@ export class PostHogAPIClient { signals?: Signal[]; }; return { - report: data.report ?? null, + report: data.report ? withNormalizedReportCharts(data.report) : null, signals: data.signals ?? [], }; } catch (error) { @@ -4269,7 +4344,7 @@ export class PostHogAPIClient { throw new Error(errorText || "Failed to update signal report state"); } - return (await response.json()) as SignalReport; + return withNormalizedReportCharts((await response.json()) as SignalReport); } /** @@ -6510,6 +6585,33 @@ export class PostHogAPIClient { return { results: data.results ?? [], columns: data.columns ?? [] }; } + /** + * Executes a query node (an `InsightVizNode`, `DataVisualizationNode`, …) + * against the team's project and returns the raw response for the caller to + * shape. Backs report-chart rendering, where the node comes from the report + * payload rather than being built here — callers must have validated the kind + * first (`normalizeReportCharts` is the gate on the inbox path). As with + * `runHogQLQuery`, a 200 carrying an `error` field is surfaced as a throw. + */ + async runInsightQuery( + query: Record, + ): Promise> { + const teamId = await this.getTeamId(); + const path = `/api/projects/${teamId}/query/`; + const url = new URL(`${this.api.baseUrl}${path}`); + const response = await this.api.fetcher.fetch({ + method: "post", + url, + path, + overrides: { body: JSON.stringify({ query }) }, + }); + const data = (await response.json()) as Record; + if (typeof data.error === "string" && data.error) { + throw new Error(data.error); + } + return data; + } + /** * Agent observability rollup over the agents' `$ai_*` events — KPIs (spend, * sessions, failure rate, p95), a 14-day daily trend + WoW deltas, and diff --git a/packages/core/src/inbox/inboxQuery.ts b/packages/core/src/inbox/inboxQuery.ts index 4eefc985f6..0063f0000f 100644 --- a/packages/core/src/inbox/inboxQuery.ts +++ b/packages/core/src/inbox/inboxQuery.ts @@ -22,6 +22,24 @@ export const inboxReportKeys = { [...inboxReportKeys.all, reportId, "artefacts"] as const, signals: (reportId: string) => [...inboxReportKeys.all, reportId, "signals"] as const, + /** + * `queryHash` keeps a revised chart query from reading the previous query's + * cached result — the `chart_id` stays the same across such an edit. + */ + chart: ( + projectId: number | null, + reportId: string, + chartId: string, + queryHash: string, + ) => + [ + ...inboxReportKeys.all, + reportId, + "chart", + chartId, + queryHash, + projectId ?? "no-project", + ] as const, availableSuggestedReviewers: (authIdentity: string | null) => [ ...inboxReportKeys.all, diff --git a/packages/core/src/inbox/reportChartPlacement.test.ts b/packages/core/src/inbox/reportChartPlacement.test.ts new file mode 100644 index 0000000000..7d0d3d0de5 --- /dev/null +++ b/packages/core/src/inbox/reportChartPlacement.test.ts @@ -0,0 +1,160 @@ +import { describe, expect, it } from "vitest"; +import { layoutReportSummary } from "./reportChartPlacement"; + +/** Compact view of a layout, so expectations read like the rendered output. */ +function render(summary: string, chartIds: string[]): string[] { + return layoutReportSummary(summary, chartIds).segments.map((segment) => + segment.kind === "chart" + ? `[chart:${segment.chartId}]` + : segment.content.trim(), + ); +} + +describe("layoutReportSummary", () => { + it("places a chart at the paragraph that references it", () => { + const summary = [ + "Signups fell over the weekend.", + "", + "[Daily signups](chart:signups)", + "", + "The drop tracks a deploy on Friday.", + ].join("\n"); + + expect(render(summary, ["signups"])).toEqual([ + "Signups fell over the weekend.", + "[chart:signups]", + "The drop tracks a deploy on Friday.", + ]); + expect(layoutReportSummary(summary, ["signups"]).placedChartIds).toEqual([ + "signups", + ]); + }); + + it("places several charts written as one paragraph, in order", () => { + const summary = "[A](chart:a)\n[B](chart:b)"; + + expect(render(summary, ["a", "b"])).toEqual(["[chart:a]", "[chart:b]"]); + }); + + it("places several charts written on one line", () => { + const summary = "[A](chart:a) [B](chart:b)"; + + expect(render(summary, ["a", "b"])).toEqual(["[chart:a]", "[chart:b]"]); + }); + + it("leaves markdown-only summaries untouched", () => { + const summary = "**What's happening**\n\nNothing to draw here."; + + const layout = layoutReportSummary(summary, []); + expect(layout.segments).toEqual([{ kind: "markdown", content: summary }]); + expect(layout.placedChartIds).toEqual([]); + }); + + it.each([ + { + name: "inside a sentence", + summary: "Signups fell, see [the chart](chart:a) for detail.", + }, + { + name: "in a list item", + summary: "- [Daily signups](chart:a)", + }, + { + name: "in a table cell", + summary: "| metric |\n| --- |\n| [Daily signups](chart:a) |", + }, + { + name: "in a blockquote", + summary: "> [Daily signups](chart:a)", + }, + { + name: "in a fenced code block", + summary: "```md\n[Daily signups](chart:a)\n```", + }, + { + name: "in an indented code block", + summary: " [Daily signups](chart:a)", + }, + { + name: "sharing a paragraph with prose below it", + summary: "[Daily signups](chart:a)\nand signups fell.", + }, + { + name: "sharing a paragraph with prose above it", + summary: "Signups fell.\n[Daily signups](chart:a)", + }, + ])("does not place a reference $name", ({ summary }) => { + const layout = layoutReportSummary(summary, ["a"]); + + expect(layout.placedChartIds).toEqual([]); + expect(layout.segments).toEqual([{ kind: "markdown", content: summary }]); + }); + + it("places only the first reference to an id, keeping later labels as text", () => { + const summary = "[First](chart:a)\n\nProse.\n\n[Again](chart:a)"; + + const layout = layoutReportSummary(summary, ["a"]); + expect(layout.placedChartIds).toEqual(["a"]); + expect(render(summary, ["a"])).toEqual([ + "[chart:a]", + "Prose.\n\n[Again](chart:a)", + ]); + }); + + it("keeps the label of a reference with no matching chart", () => { + const summary = "[Missing](chart:gone)"; + + expect(render(summary, ["a"])).toEqual(["[Missing](chart:gone)"]); + }); + + it("keeps unplaceable labels beside a chart placed on the same line", () => { + const summary = "[Missing](chart:gone) [Real](chart:a)"; + + expect(render(summary, ["a"])).toEqual([ + "[Missing](chart:gone)", + "[chart:a]", + ]); + }); + + it("ignores a reference whose id is not a valid chart id", () => { + const summary = "[Shouty](chart:NOT_VALID)"; + + expect(render(summary, ["NOT_VALID"])).toEqual([ + "[Shouty](chart:NOT_VALID)", + ]); + }); + + it("tolerates a link title and line-break tags around a reference", () => { + const summary = '[A](chart:a "Daily signups")
'; + + expect(render(summary, ["a"])).toEqual(["[chart:a]"]); + }); + + it.each([ + { + name: "a link-reference definition", + summary: + "See [the docs][1].\n\n[Chart](chart:a)\n\n[1]: https://posthog.com", + }, + { + name: "a footnote definition", + summary: "Signups fell[^1].\n\n[Chart](chart:a)\n\n[^1]: Since Friday.", + }, + ])( + "skips inline placement when the summary carries $name, keeping the prose whole", + ({ summary }) => { + // Definitions resolve document-wide, so splitting would strand them in a + // separate render pass and the reference would read as literal text. + const layout = layoutReportSummary(summary, ["a"]); + + expect(layout.placedChartIds).toEqual([]); + expect(layout.segments).toEqual([{ kind: "markdown", content: summary }]); + }, + ); + + it("reports nothing placed when the summary references no chart", () => { + const layout = layoutReportSummary("Just prose.", ["a", "b"]); + + expect(layout.placedChartIds).toEqual([]); + }); +}); diff --git a/packages/core/src/inbox/reportChartPlacement.ts b/packages/core/src/inbox/reportChartPlacement.ts new file mode 100644 index 0000000000..379f0b2437 --- /dev/null +++ b/packages/core/src/inbox/reportChartPlacement.ts @@ -0,0 +1,187 @@ +import { matchReportChartRef } from "@posthog/shared"; + +/** + * Splits a report summary into the pieces a detail view renders in order: + * prose, and the charts the prose places. + * + * A summary places a chart with a `[label](chart:)` link (see + * `packages/shared/src/report-charts.ts`). Rather than teaching the markdown + * renderer to swap an inline anchor for a block-level chart — which would nest a + * chart inside a `

` — we cut the summary at the paragraphs that exist purely + * to hold chart references and hand the remaining prose to the renderer + * unchanged. Splitting only at those paragraphs keeps every other markdown + * construct (lists, tables, fenced code) inside a single render pass. + * + * Placement rules, matching web: + * - Only a paragraph consisting solely of chart references places charts. A + * reference inside a sentence, a list item, a table cell, or a code fence + * stays as its label text. + * - The first reference to an id places it; later ones fall back to their label. + * - Ids with no matching chart fall back to their label too, so a stale + * reference reads as text rather than vanishing. + * - A summary carrying link-reference or footnote definitions isn't split at all + * (see `REFERENCE_DEFINITION`); its charts all render after the prose. + */ + +export type ReportSummarySegment = + | { kind: "markdown"; content: string } + | { kind: "chart"; chartId: string }; + +export interface ReportSummaryLayout { + segments: ReportSummarySegment[]; + /** Ids placed inline, in placement order. The rest render after the summary. */ + placedChartIds: string[]; +} + +/** + * A markdown inline link whose target is a chart reference, with an optional + * link title. Labels containing `]` aren't supported — neither is the reference + * generator that writes them. + */ +const CHART_LINK = /\[([^\]]*)\]\(\s*(chart:[^\s)"]+)\s*(?:"[^"]*"\s*)?\)/g; + +/** Markdown treats four leading spaces as an indented code block. */ +const MAX_LEADING_SPACES = 3; + +/** Tolerated between references on a placement line, alongside whitespace. */ +const LINE_BREAK_TAGS = //gi; + +/** Opens or closes a fenced code block. */ +const FENCE = /^ {0,3}(?:`{3,}|~{3,})/; + +/** + * A link-reference or footnote definition (`[1]: …`, `[^note]: …`). These resolve + * document-wide, so a definition and its use can sit either side of a chart — + * and splitting the summary would strand them in separate render passes, leaving + * the reference as literal `\[label]\[1]` text. Rare enough in report summaries + * that skipping inline placement entirely is the right trade: charts still render + * after the prose, and the prose stays intact. + */ +const REFERENCE_DEFINITION = /^ {0,3}\[[^\]]+]:/m; + +interface ChartReference { + /** The full `[label](chart:id)` source, used when the reference can't place. */ + source: string; + chartId: string | null; +} + +/** + * The chart references on a line, but only when the line holds nothing else — + * that is the signal the paragraph exists to place charts. Null for any other + * line, including blank ones. + */ +function matchPlacementLine(line: string): ChartReference[] | null { + const leadingSpaces = line.length - line.trimStart().length; + if (leadingSpaces > MAX_LEADING_SPACES) return null; + + const trimmed = line.trim(); + if (!trimmed) return null; + + const references: ChartReference[] = []; + const remainder = trimmed + .replace(CHART_LINK, (source, _label: string, target: string) => { + references.push({ source, chartId: matchReportChartRef(target) }); + return ""; + }) + .replace(LINE_BREAK_TAGS, ""); + + if (references.length === 0 || remainder.trim().length > 0) return null; + return references; +} + +/** + * Per-line chart references, set only for lines belonging to a paragraph whose + * every line is a placement line. Paragraph = a maximal run of non-blank lines + * outside any code fence. + */ +function findPlacementParagraphs(lines: string[]): (ChartReference[] | null)[] { + const candidates = lines.map((line) => matchPlacementLine(line)); + const resolved: (ChartReference[] | null)[] = lines.map(() => null); + + let inFence = false; + let paragraphStart: number | null = null; + let paragraphPlaces = true; + + const closeParagraph = (end: number) => { + if (paragraphStart !== null && paragraphPlaces) { + for (let i = paragraphStart; i < end; i++) resolved[i] = candidates[i]; + } + paragraphStart = null; + paragraphPlaces = true; + }; + + for (const [index, line] of lines.entries()) { + if (FENCE.test(line)) { + closeParagraph(index); + inFence = !inFence; + continue; + } + if (inFence || line.trim() === "") { + closeParagraph(index); + continue; + } + if (paragraphStart === null) paragraphStart = index; + if (!candidates[index]) paragraphPlaces = false; + } + closeParagraph(lines.length); + + return resolved; +} + +export function layoutReportSummary( + summary: string, + chartIds: readonly string[], +): ReportSummaryLayout { + if (REFERENCE_DEFINITION.test(summary)) { + return { + segments: [{ kind: "markdown", content: summary }], + placedChartIds: [], + }; + } + + const available = new Set(chartIds); + const segments: ReportSummarySegment[] = []; + const placedChartIds: string[] = []; + const placed = new Set(); + + const lines = summary.split("\n"); + const placements = findPlacementParagraphs(lines); + + /** Prose accumulated since the last chart, flushed as one markdown segment. */ + let buffer: string[] = []; + const flushBuffer = () => { + const content = buffer.join("\n"); + buffer = []; + if (content.trim()) segments.push({ kind: "markdown", content }); + }; + + for (const [index, line] of lines.entries()) { + const references = placements[index]; + if (!references) { + buffer.push(line); + continue; + } + + /** References this line couldn't place, kept as text in reading order. */ + const unplaceable: string[] = []; + for (const { source, chartId } of references) { + if (!chartId || !available.has(chartId) || placed.has(chartId)) { + unplaceable.push(source); + continue; + } + // Prose first, then any labels preceding the chart on this line. + if (unplaceable.length > 0) { + buffer.push(unplaceable.join(" ")); + unplaceable.length = 0; + } + flushBuffer(); + placed.add(chartId); + placedChartIds.push(chartId); + segments.push({ kind: "chart", chartId }); + } + if (unplaceable.length > 0) buffer.push(unplaceable.join(" ")); + } + + flushBuffer(); + return { segments, placedChartIds }; +} diff --git a/packages/core/src/inbox/reportCharts.test.ts b/packages/core/src/inbox/reportCharts.test.ts new file mode 100644 index 0000000000..1cdaf675a8 --- /dev/null +++ b/packages/core/src/inbox/reportCharts.test.ts @@ -0,0 +1,275 @@ +import type { ReportChart } from "@posthog/shared"; +import { describe, expect, it } from "vitest"; +import { + planReportChart, + reportChartQueryHash, + resolveReportChartSize, + shapeNumberResponse, + shapeTableResponse, + shapeTrendsResponse, +} from "./reportCharts"; + +function chart(query: Record, extra?: Partial) { + return { + chart_id: "c", + title: "Chart", + query, + ...extra, + } satisfies ReportChart; +} + +function trends( + source: Record = { kind: "TrendsQuery" }, +): Record { + return { kind: "InsightVizNode", source }; +} + +describe("planReportChart", () => { + it.each([ + { display: undefined, expected: { kind: "timeseries", variant: "line" } }, + { + display: "ActionsLineGraph", + expected: { kind: "timeseries", variant: "line" }, + }, + { + display: "ActionsAreaGraph", + expected: { kind: "timeseries", variant: "line" }, + }, + { display: "ActionsBar", expected: { kind: "timeseries", variant: "bar" } }, + { + display: "ActionsStackedBar", + expected: { kind: "timeseries", variant: "bar" }, + }, + { display: "BoldNumber", expected: { kind: "number" } }, + ])("plans a $display trend", ({ display, expected }) => { + const query = trends({ + kind: "TrendsQuery", + ...(display ? { trendsFilter: { display } } : {}), + }); + + expect(planReportChart(chart(query))).toEqual(expected); + }); + + it("plans a SQL query as a table", () => { + expect(planReportChart(chart({ kind: "DataVisualizationNode" }))).toEqual({ + kind: "table", + }); + }); + + it.each([ + { query: trends({ kind: "FunnelsQuery" }), reason: "Funnel charts" }, + { query: trends({ kind: "RetentionQuery" }), reason: "Retention charts" }, + { query: { kind: "SavedInsightNode" }, reason: "Saved insights" }, + { + query: trends({ + kind: "TrendsQuery", + trendsFilter: { display: "WorldMap" }, + }), + reason: "isn't supported", + }, + { query: trends({ kind: "SomeFutureQuery" }), reason: "isn't supported" }, + ])("marks $reason unsupported", ({ query, reason }) => { + const plan = planReportChart(chart(query)); + + expect(plan.kind).toBe("unsupported"); + expect(plan.kind === "unsupported" && plan.reason).toContain(reason); + }); +}); + +describe("resolveReportChartSize", () => { + it("honours the size the report chose", () => { + const plan = { kind: "number" } as const; + + expect( + resolveReportChartSize(chart(trends(), { size: "large" }), plan), + ).toBe("large"); + }); + + it.each([ + { plan: { kind: "number" } as const, expected: "small" }, + { plan: { kind: "table" } as const, expected: "medium" }, + { + plan: { kind: "timeseries", variant: "line" } as const, + expected: "medium", + }, + ])("defaults a $plan.kind chart to $expected", ({ plan, expected }) => { + expect(resolveReportChartSize(chart(trends()), plan)).toBe(expected); + }); +}); + +describe("shapeTrendsResponse", () => { + it("maps results into parallel series and labels", () => { + const response = { + results: [ + { label: "$pageview", data: [1, 2, 3], days: ["d1", "d2", "d3"] }, + { label: "signup", data: [4, 5, 6], days: ["d1", "d2", "d3"] }, + ], + }; + + expect(shapeTrendsResponse(response)).toEqual({ + labels: ["d1", "d2", "d3"], + series: [ + { key: "0-$pageview", label: "$pageview", data: [1, 2, 3] }, + { key: "1-signup", label: "signup", data: [4, 5, 6] }, + ], + }); + }); + + it("falls back to `labels` when the trend has no days", () => { + const response = { results: [{ data: [1, 2], labels: ["a", "b"] }] }; + + expect(shapeTrendsResponse(response)?.labels).toEqual(["a", "b"]); + }); + + it("names an unlabelled series by position", () => { + const response = { results: [{ data: [1], days: ["d1"] }] }; + + expect(shapeTrendsResponse(response)?.series[0].label).toBe("Series 1"); + }); + + it("trims every series to the shortest run so the x-axis stays aligned", () => { + const response = { + results: [ + { label: "a", data: [1, 2, 3], days: ["d1", "d2", "d3"] }, + { label: "b", data: [4], days: ["d1", "d2", "d3"] }, + ], + }; + + expect(shapeTrendsResponse(response)).toEqual({ + labels: ["d1"], + series: [ + { key: "0-a", label: "a", data: [1] }, + { key: "1-b", label: "b", data: [4] }, + ], + }); + }); + + it("skips an empty series instead of discarding the whole chart", () => { + const response = { + results: [ + { label: "a", data: [1, 2], days: ["d1", "d2"] }, + { label: "empty", data: [] }, + ], + }; + + expect(shapeTrendsResponse(response)).toEqual({ + labels: ["d1", "d2"], + series: [{ key: "0-a", label: "a", data: [1, 2] }], + }); + }); + + it("drops a series with a non-numeric datapoint rather than reading it as zero", () => { + const response = { + results: [ + { label: "gappy", data: [1, null, 3], days: ["d1", "d2", "d3"] }, + { label: "solid", data: [4, 5, 6], days: ["d1", "d2", "d3"] }, + ], + }; + + expect(shapeTrendsResponse(response)?.series).toEqual([ + { key: "1-solid", label: "solid", data: [4, 5, 6] }, + ]); + }); + + it.each([ + { name: "no results", response: { results: [] } }, + { name: "no labels", response: { results: [{ data: [1] }] } }, + { name: "no data", response: { results: [{ days: ["d1"] }] } }, + { name: "a non-object response", response: null }, + { + name: "only non-numeric series", + response: { results: [{ data: ["1"], days: ["d1"] }] }, + }, + ])("returns null for $name", ({ response }) => { + expect(shapeTrendsResponse(response)).toBeNull(); + }); +}); + +describe("reportChartQueryHash", () => { + it("is stable for the same query", () => { + const query = { kind: "InsightVizNode", source: { kind: "TrendsQuery" } }; + + expect(reportChartQueryHash(query)).toBe( + reportChartQueryHash({ ...query }), + ); + }); + + it("changes when the query changes", () => { + const before = reportChartQueryHash({ + kind: "InsightVizNode", + series: [1], + }); + const after = reportChartQueryHash({ kind: "InsightVizNode", series: [2] }); + + expect(before).not.toBe(after); + }); +}); + +describe("shapeNumberResponse", () => { + it("prefers the aggregate the backend computed", () => { + const response = { results: [{ aggregated_value: 42, label: "Signups" }] }; + + expect(shapeNumberResponse(response)).toEqual({ + value: 42, + label: "Signups", + }); + }); + + it("falls back to count, then to summing the series", () => { + expect(shapeNumberResponse({ results: [{ count: 7 }] })?.value).toBe(7); + expect(shapeNumberResponse({ results: [{ data: [1, 2, 3] }] })?.value).toBe( + 6, + ); + }); + + it("returns null when there is no figure to show", () => { + expect(shapeNumberResponse({ results: [{}] })).toBeNull(); + expect(shapeNumberResponse({ results: [] })).toBeNull(); + }); +}); + +describe("shapeTableResponse", () => { + it("prints a result grid", () => { + const response = { + columns: ["event", "count"], + results: [ + ["$pageview", 10], + ["signup", null], + ], + }; + + expect(shapeTableResponse(response)).toEqual({ + columns: ["event", "count"], + rows: [ + ["$pageview", "10"], + ["signup", "—"], + ], + truncatedRows: 0, + }); + }); + + it("names columns by position when the response has none", () => { + const response = { results: [["a", "b"]] }; + + expect(shapeTableResponse(response)?.columns).toEqual([ + "Column 1", + "Column 2", + ]); + }); + + it("caps rows and reports how many it dropped", () => { + const response = { + columns: ["n"], + results: Array.from({ length: 105 }, (_entry, index) => [index]), + }; + + const shaped = shapeTableResponse(response); + expect(shaped?.rows).toHaveLength(100); + expect(shaped?.truncatedRows).toBe(5); + }); + + it("returns null when there is no grid", () => { + expect(shapeTableResponse({ results: null })).toBeNull(); + expect(shapeTableResponse(null)).toBeNull(); + }); +}); diff --git a/packages/core/src/inbox/reportCharts.ts b/packages/core/src/inbox/reportCharts.ts new file mode 100644 index 0000000000..e7414d4807 --- /dev/null +++ b/packages/core/src/inbox/reportCharts.ts @@ -0,0 +1,260 @@ +import type { ReportChart, ReportChartSize } from "@posthog/shared"; + +/** + * Decides what a report chart's query can be drawn as here, and reshapes the + * `/query/` response into the plain series/rows a renderer needs. + * + * Web draws these with the full insight-visualization stack; the desktop app has + * a charting library but no insight renderer, so it covers the query shapes that + * map cleanly onto it and says so plainly for the rest. An unsupported chart + * still shows its title, caption, and a link out — the evidence stays visible + * even when the picture can't be drawn locally. + */ + +export type ReportChartPlan = + | { kind: "timeseries"; variant: "line" | "bar" } + | { kind: "number" } + | { kind: "table" } + | { kind: "unsupported"; reason: string }; + +/** Trends displays that map onto a time-series chart, by variant. */ +const LINE_DISPLAYS = new Set([ + "ActionsLineGraph", + "ActionsLineGraphCumulative", + "ActionsAreaGraph", +]); +const BAR_DISPLAYS = new Set(["ActionsBar", "ActionsStackedBar"]); + +/** Insight query kinds we can't draw, with the label used to explain why. */ +const INSIGHT_KIND_LABELS: Record = { + FunnelsQuery: "Funnel", + RetentionQuery: "Retention", + PathsQuery: "Paths", + StickinessQuery: "Stickiness", + LifecycleQuery: "Lifecycle", + CalendarHeatmapQuery: "Calendar heatmap", + FunnelCorrelationQuery: "Funnel correlation", +}; + +function asRecord(value: unknown): Record | null { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return null; + } + return value as Record; +} + +function unsupported(reason: string): ReportChartPlan { + return { kind: "unsupported", reason }; +} + +export function planReportChart(chart: ReportChart): ReportChartPlan { + const query = chart.query; + + if (query.kind === "DataVisualizationNode") return { kind: "table" }; + + if (query.kind === "SavedInsightNode") { + return unsupported("Saved insights open in PostHog."); + } + + if (query.kind !== "InsightVizNode") { + return unsupported("This chart type isn't supported here yet."); + } + + const source = asRecord(query.source); + const sourceKind = typeof source?.kind === "string" ? source.kind : null; + if (sourceKind !== "TrendsQuery") { + const label = sourceKind ? INSIGHT_KIND_LABELS[sourceKind] : null; + return unsupported( + label + ? `${label} charts open in PostHog.` + : "This chart type isn't supported here yet.", + ); + } + + const display = asRecord(source?.trendsFilter)?.display; + if (typeof display !== "string" || LINE_DISPLAYS.has(display)) { + // Trends default to a line graph when no display is set. + return { kind: "timeseries", variant: "line" }; + } + if (BAR_DISPLAYS.has(display)) return { kind: "timeseries", variant: "bar" }; + if (display === "BoldNumber") return { kind: "number" }; + return unsupported("This chart type isn't supported here yet."); +} + +/** + * A short stable digest of a chart's query, for cache keys. A report can revise + * a chart's query while keeping its `chart_id` — keying the cached result on the + * id alone would keep serving the old query's numbers under the new query's + * title. djb2 over the serialized node: collisions don't matter here, only that + * the same query always maps to the same key. + */ +export function reportChartQueryHash(query: Record): string { + const serialized = JSON.stringify(query); + let hash = 5381; + for (let index = 0; index < serialized.length; index++) { + hash = ((hash << 5) + hash + serialized.charCodeAt(index)) | 0; + } + return (hash >>> 0).toString(36); +} + +/** Height bucket for a chart, honouring the report's choice when it made one. */ +export function resolveReportChartSize( + chart: ReportChart, + plan: ReportChartPlan, +): ReportChartSize { + if (chart.size) return chart.size; + return plan.kind === "number" ? "small" : "medium"; +} + +export interface ReportChartSeries { + key: string; + label: string; + data: number[]; +} + +export interface ReportChartTimeseries { + series: ReportChartSeries[]; + /** X-axis categories — ISO days for a dated trend, bucket labels otherwise. */ + labels: string[]; +} + +/** + * Datapoints, only if every entry really is one. Coercing a null or a string to + * 0 would draw a dip the backend never reported, so an unexpected entry + * disqualifies the series instead — a missing series is honest, an invented zero + * is not. + */ +function numericArray(value: unknown): number[] | null { + if (!Array.isArray(value)) return null; + const numbers: number[] = []; + for (const entry of value) { + if (typeof entry !== "number" || !Number.isFinite(entry)) return null; + numbers.push(entry); + } + return numbers; +} + +function stringArray(value: unknown): string[] | null { + if (!Array.isArray(value) || value.length === 0) return null; + return value.map((entry) => (typeof entry === "string" ? entry : "")); +} + +function responseResults(response: unknown): unknown[] { + const results = asRecord(response)?.results; + return Array.isArray(results) ? results : []; +} + +/** + * Trends `/query/` results into parallel series + labels. Series are trimmed to + * the shortest run so a partial series can't shift the x-axis; a response with + * no usable series returns null so the caller can show an empty state. + */ +export function shapeTrendsResponse( + response: unknown, +): ReportChartTimeseries | null { + const entries = responseResults(response) + .map((entry) => asRecord(entry)) + .filter((entry): entry is Record => entry !== null); + if (entries.length === 0) return null; + + const first = entries[0]; + const labels = stringArray(first.days) ?? stringArray(first.labels); + if (!labels) return null; + + const series: ReportChartSeries[] = []; + for (const [index, entry] of entries.entries()) { + const data = numericArray(entry.data); + // An empty series carries nothing to draw; skipping it keeps it from + // collapsing the shared x-axis the other series are plotted against. + if (!data || data.length === 0) continue; + const label = + (typeof entry.label === "string" && entry.label.trim()) || + `Series ${index + 1}`; + series.push({ key: `${index}-${label}`, label, data }); + } + if (series.length === 0) return null; + + const length = series.reduce( + (shortest, entry) => Math.min(shortest, entry.data.length), + labels.length, + ); + if (length === 0) return null; + + return { + labels: labels.slice(0, length), + series: series.map((entry) => ({ + ...entry, + data: entry.data.slice(0, length), + })), + }; +} + +/** + * The single figure behind a BoldNumber trend. Prefers the aggregate the backend + * computed, falling back to summing the series it returned. + */ +export function shapeNumberResponse( + response: unknown, +): { value: number; label: string } | null { + const first = asRecord(responseResults(response)[0]); + if (!first) return null; + + const label = + (typeof first.label === "string" && first.label.trim()) || "Total"; + + for (const key of ["aggregated_value", "count"] as const) { + const value = first[key]; + if (typeof value === "number" && Number.isFinite(value)) { + return { value, label }; + } + } + + const data = numericArray(first.data); + if (!data || data.length === 0) return null; + return { value: data.reduce((sum, entry) => sum + entry, 0), label }; +} + +/** Rows kept for a SQL-backed chart; beyond this the chart stops being a chart. */ +export const MAX_REPORT_CHART_TABLE_ROWS = 100; + +export interface ReportChartTable { + columns: string[]; + rows: string[][]; + /** Rows dropped past the cap, surfaced so the table doesn't lie by omission. */ + truncatedRows: number; +} + +function formatCell(value: unknown): string { + if (value === null || value === undefined) return "—"; + if (typeof value === "string") return value; + if (typeof value === "number" || typeof value === "boolean") { + return String(value); + } + return JSON.stringify(value) ?? ""; +} + +/** A `HogQLQuery` result grid into printable columns and rows. */ +export function shapeTableResponse(response: unknown): ReportChartTable | null { + const record = asRecord(response); + if (!record) return null; + + const rawRows = Array.isArray(record.results) ? record.results : null; + if (!rawRows) return null; + + const columns = stringArray(record.columns) ?? []; + const rows = rawRows + .slice(0, MAX_REPORT_CHART_TABLE_ROWS) + .map((row) => + Array.isArray(row) ? row.map(formatCell) : [formatCell(row)], + ); + if (columns.length === 0 && rows.length === 0) return null; + + return { + columns: + columns.length > 0 + ? columns + : (rows[0] ?? []).map((_cell, index) => `Column ${index + 1}`), + rows, + truncatedRows: Math.max(0, rawRows.length - rows.length), + }; +} diff --git a/packages/shared/src/domain-types.ts b/packages/shared/src/domain-types.ts index 8f1e8dc56d..55434a480a 100644 --- a/packages/shared/src/domain-types.ts +++ b/packages/shared/src/domain-types.ts @@ -469,7 +469,10 @@ export interface DetectedApplication { icon?: string; // Base64 data URL } +import type { ReportChart } from "./report-charts"; import type { SignalReportStatus } from "./signal-types"; + +export type { ReportChart, ReportChartSize } from "./report-charts"; export type { SignalReportStatus }; /** Actionability priority from the researched report (actionability judgment artefact). */ @@ -519,6 +522,12 @@ export interface SignalReport { source_products?: string[]; /** PR URL from the latest implementation task run, if available. */ implementation_pr_url?: string | null; + /** + * Charts the report shows, in the order they were written. The summary places + * one with a `[label](chart:)` link; the rest render below it. + * Absent on reports authored before charts shipped. + */ + charts?: ReportChart[]; } export interface SignalReportArtefactContent { diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index f6d15bc1ae..287e93df8a 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -127,6 +127,8 @@ export { type CloudTaskStatusUpdate, type CloudTaskUpdatePayload, isTerminalStatus, + type ReportChart, + type ReportChartSize, type SignalReportPriority, type Task, type TaskRun, @@ -276,6 +278,16 @@ export { type RegionLabel, } from "./regions"; export { normalizeRepoKey } from "./repo"; +export { + hasForbiddenReportChartQueryNode, + isReportChartId, + isReportChartQueryKind, + isReportChartSize, + MAX_REPORT_CHART_CAPTION_LENGTH, + MAX_REPORT_CHART_TITLE_LENGTH, + MAX_REPORT_CHARTS, + matchReportChartRef, +} from "./report-charts"; export { getTaskRepository, parseRepository } from "./repository"; export { Saga, diff --git a/packages/shared/src/report-charts.ts b/packages/shared/src/report-charts.ts new file mode 100644 index 0000000000..432ac7c8ef --- /dev/null +++ b/packages/shared/src/report-charts.ts @@ -0,0 +1,131 @@ +/** + * Contract for the charts a Self-driving report carries, mirroring the backend + * `ReportChart` (`products/signals/backend/report_charts.py`, PostHog/posthog#73733). + * + * A report summary places a chart inline with a `[label](chart:)` + * link; charts nothing references render after the prose. The limits below are + * the backend's, re-declared here because we re-validate on read: a chart's + * `query` is POSTed back to `/api/projects/:id/query/` to draw it, so the + * client decides for itself which query nodes it is willing to execute rather + * than trusting the payload. + */ + +/** Markdown link scheme a summary uses to place a chart. */ +const REPORT_CHART_REF_PREFIX = "chart:"; + +/** `chart_id` shape the backend accepts: lowercase slug of letters, digits, `_`, `-`. */ +const REPORT_CHART_ID_PATTERN = /^[a-z0-9][a-z0-9_-]*$/; + +export const MAX_REPORT_CHARTS = 20; +const MAX_REPORT_CHART_ID_LENGTH = 100; +export const MAX_REPORT_CHART_TITLE_LENGTH = 200; +export const MAX_REPORT_CHART_CAPTION_LENGTH = 500; + +/** + * Query node kinds a report chart may carry. The backend validates writes + * against the same set; anything else is dropped on read instead of executed. + */ +const REPORT_CHART_QUERY_KINDS = [ + "InsightVizNode", + "DataVisualizationNode", + "SavedInsightNode", +] as const; + +type ReportChartQueryKind = (typeof REPORT_CHART_QUERY_KINDS)[number]; + +/** + * Query kinds that run caller-supplied code server-side. The backend refuses to + * store a chart with one of these nested anywhere in its query; we refuse to + * keep one, so such a query never reaches the query endpoint from here either. + */ +const FORBIDDEN_QUERY_KINDS = new Set(["HogQuery", "SuggestedQuestionsQuery"]); + +/** Object keys that smuggle executable payloads into an otherwise benign node. */ +const FORBIDDEN_QUERY_KEYS = new Set(["bytecode", "sendRawQuery"]); + +/** Guards the recursive scan against a pathologically nested payload. */ +const MAX_QUERY_DEPTH = 100; + +export type ReportChartSize = "small" | "medium" | "large"; + +const REPORT_CHART_SIZES: readonly ReportChartSize[] = [ + "small", + "medium", + "large", +]; + +/** One chart attached to a report — drawn inline in the summary or below it. */ +export interface ReportChart { + chart_id: string; + title: string; + /** Query node to render. `kind` is one of `REPORT_CHART_QUERY_KINDS`. */ + query: Record; + caption?: string | null; + size?: ReportChartSize | null; +} + +export function isReportChartId(value: string): boolean { + return ( + value.length > 0 && + value.length <= MAX_REPORT_CHART_ID_LENGTH && + REPORT_CHART_ID_PATTERN.test(value) + ); +} + +export function isReportChartSize(value: unknown): value is ReportChartSize { + return ( + typeof value === "string" && + REPORT_CHART_SIZES.includes(value as ReportChartSize) + ); +} + +export function isReportChartQueryKind( + value: unknown, +): value is ReportChartQueryKind { + return ( + typeof value === "string" && + REPORT_CHART_QUERY_KINDS.includes(value as ReportChartQueryKind) + ); +} + +/** + * The `chart_id` a markdown link target references, or null when the href isn't + * a chart reference. Used both to place charts while splitting a summary and to + * keep `chart:` hrefs from rendering as broken anchors. + */ +export function matchReportChartRef( + href: string | null | undefined, +): string | null { + if (!href || !href.startsWith(REPORT_CHART_REF_PREFIX)) return null; + const chartId = href.slice(REPORT_CHART_REF_PREFIX.length); + return isReportChartId(chartId) ? chartId : null; +} + +/** + * Whether a query node carries an executable payload anywhere inside it. A + * chart's own `kind` is checked separately; this catches one nested in a + * `source`, a series entry, or a filter. + */ +export function hasForbiddenReportChartQueryNode( + value: unknown, + depth = 0, +): boolean { + if (depth > MAX_QUERY_DEPTH) return true; + if (Array.isArray(value)) { + return value.some((entry) => + hasForbiddenReportChartQueryNode(entry, depth + 1), + ); + } + if (typeof value !== "object" || value === null) return false; + + const record = value as Record; + for (const key of Object.keys(record)) { + if (FORBIDDEN_QUERY_KEYS.has(key)) return true; + } + if (typeof record.kind === "string" && FORBIDDEN_QUERY_KINDS.has(record.kind)) + return true; + + return Object.values(record).some((entry) => + hasForbiddenReportChartQueryNode(entry, depth + 1), + ); +} diff --git a/packages/ui/src/features/editor/components/MarkdownRenderer.tsx b/packages/ui/src/features/editor/components/MarkdownRenderer.tsx index 32a43862a0..87bf3fb0c0 100644 --- a/packages/ui/src/features/editor/components/MarkdownRenderer.tsx +++ b/packages/ui/src/features/editor/components/MarkdownRenderer.tsx @@ -1,4 +1,4 @@ -import { isPostHogCodeDeeplink } from "@posthog/shared"; +import { isPostHogCodeDeeplink, matchReportChartRef } from "@posthog/shared"; import { GithubRefChip } from "@posthog/ui/features/editor/components/GithubRefChip"; import { parseGithubIssueUrl } from "@posthog/ui/features/message-editor/githubIssueUrl"; import { CodeBlock } from "@posthog/ui/primitives/CodeBlock"; @@ -29,6 +29,9 @@ function preprocessMarkdown(content: string): string { function markdownUrlTransform(value: string): string { if (isPostHogCodeDeeplink(value)) return value; + // Sanitizing would strip `chart:` as an unknown protocol, leaving the `a` + // override no href to recognise the reference by. + if (matchReportChartRef(value)) return value; return defaultUrlTransform(value); } @@ -75,6 +78,12 @@ export const baseComponents: Components = { {children} ), a: ({ href, children }) => { + // `chart:` targets a chart attached to a Self-driving report, placed by + // the detail view (see `layoutReportSummary`). Anywhere else — and for a + // reference the detail view couldn't place — the label reads as plain text, + // since `chart:` is not a scheme a browser can follow. + if (matchReportChartRef(href)) return <>{children}; + const githubRef = href ? parseGithubIssueUrl(href) : null; if (githubRef) { const isAutoLink = typeof children === "string" && children === href; diff --git a/packages/ui/src/features/inbox/components/InboxDetailFrame.tsx b/packages/ui/src/features/inbox/components/InboxDetailFrame.tsx index bcaacd6c00..b13590d866 100644 --- a/packages/ui/src/features/inbox/components/InboxDetailFrame.tsx +++ b/packages/ui/src/features/inbox/components/InboxDetailFrame.tsx @@ -184,6 +184,8 @@ export function InboxDetailFrame({ fallback="No summary yet – the Responder is still investigating." variant="detail" pending={report.status === "in_progress"} + charts={report.charts} + reportId={report.id} /> {belowSummary} diff --git a/packages/ui/src/features/inbox/components/detail/ReportChart.tsx b/packages/ui/src/features/inbox/components/detail/ReportChart.tsx new file mode 100644 index 0000000000..479c5c16be --- /dev/null +++ b/packages/ui/src/features/inbox/components/detail/ReportChart.tsx @@ -0,0 +1,253 @@ +import { ArrowSquareOutIcon, ChartLineIcon } from "@phosphor-icons/react"; +import { + planReportChart, + type ReportChartPlan, + resolveReportChartSize, + shapeNumberResponse, + shapeTableResponse, + shapeTrendsResponse, +} from "@posthog/core/inbox/reportCharts"; +import { + type Series, + TimeSeriesBarChart, + TimeSeriesLineChart, + useChartTheme, +} from "@posthog/quill-charts"; +import type { + ReportChart as ReportChartModel, + ReportChartSize, +} from "@posthog/shared/types"; +import { useReportChartData } from "@posthog/ui/features/inbox/hooks/useReportChartData"; +import { openExternalUrl } from "@posthog/ui/shell/openExternal"; +import { inboxReportUrl } from "@posthog/ui/utils/posthogLinks"; +import { Flex, Text } from "@radix-ui/themes"; +import { useMemo } from "react"; + +interface ReportChartProps { + reportId: string; + chart: ReportChartModel; +} + +/** Body heights per size bucket, matching web's 9rem / 18rem / 28rem. */ +const HEIGHT_CLASS: Record = { + small: "h-36", + medium: "h-72", + large: "h-[28rem]", +}; + +/** + * One chart attached to a report, drawn read-only. Trends and SQL results render + * natively; anything the desktop app has no renderer for keeps its title and + * caption and offers the report in PostHog instead, so the evidence degrades + * visibly rather than disappearing. + */ +export function ReportChart({ reportId, chart }: ReportChartProps) { + const plan = useMemo(() => planReportChart(chart), [chart]); + const size = resolveReportChartSize(chart, plan); + + return ( +

+ + + + {chart.title} + + +
+ {plan.kind === "unsupported" ? ( + + ) : ( + + )} +
+ {chart.caption && ( +
+ {chart.caption} +
+ )} +
+ ); +} + +function ChartBody({ + reportId, + chart, + plan, + heightClass, +}: { + reportId: string; + chart: ReportChartModel; + plan: Exclude; + heightClass: string; +}) { + const theme = useChartTheme(); + const { data, isPending, isError, error } = useReportChartData( + reportId, + chart, + ); + + if (isPending) { + return ( +
+ ); + } + if (isError) { + return ( + + ); + } + + if (plan.kind === "number") { + const shaped = shapeNumberResponse(data); + if (!shaped) return ; + return ( + + + {shaped.value.toLocaleString()} + + {shaped.label} + + ); + } + + if (plan.kind === "table") { + const shaped = shapeTableResponse(data); + if (!shaped || shaped.rows.length === 0) return ; + return ( +
+ + + + {shaped.columns.map((column) => ( + + ))} + + + + {shaped.rows.map((row, rowIndex) => ( + // Rows carry no id, and duplicate rows are legitimate query output. + // biome-ignore lint/suspicious/noArrayIndexKey: row order is the only identity + + {row.map((cell, cellIndex) => ( + + ))} + + ))} + +
+ {column} +
+ {cell} +
+ {shaped.truncatedRows > 0 && ( + + {shaped.truncatedRows} more row + {shaped.truncatedRows === 1 ? "" : "s"} not shown. + + )} +
+ ); + } + + const shaped = shapeTrendsResponse(data); + if (!shaped) return ; + const series: Series[] = shaped.series.map((entry) => ({ + key: entry.key, + label: entry.label, + data: entry.data, + })); + const config = { + xAxis: { interval: "day" as const }, + showCrosshair: true, + showAxisLines: true, + }; + + return ( + // flex-col + fixed height: the quill chart sizes its canvas by filling a + // flex-column parent; a plain block collapses it to 0. +
+ {plan.variant === "bar" ? ( + + ) : ( + + )} +
+ ); +} + +function ChartEmpty() { + return ( + + This chart's query returned no data. + + ); +} + +/** + * The visible degradation: says why the chart isn't drawn and links to the + * report in PostHog, which renders it. + */ +function ChartUnavailable({ + reportId, + message, +}: { + reportId: string; + message: string; +}) { + const url = inboxReportUrl(reportId); + return ( + + {message} + {url && ( + + )} + + ); +} diff --git a/packages/ui/src/features/inbox/components/utils/SignalReportSummaryMarkdown.tsx b/packages/ui/src/features/inbox/components/utils/SignalReportSummaryMarkdown.tsx index 5de8eabe42..b24a9b265d 100644 --- a/packages/ui/src/features/inbox/components/utils/SignalReportSummaryMarkdown.tsx +++ b/packages/ui/src/features/inbox/components/utils/SignalReportSummaryMarkdown.tsx @@ -1,5 +1,8 @@ +import { layoutReportSummary } from "@posthog/core/inbox/reportChartPlacement"; import { formatSignalReportSummaryMarkdown } from "@posthog/core/inbox/reportPresentation"; +import type { ReportChart } from "@posthog/shared/types"; import { MarkdownRenderer } from "@posthog/ui/features/editor/components/MarkdownRenderer"; +import { ReportChart as ReportChartCard } from "@posthog/ui/features/inbox/components/detail/ReportChart"; import { Box } from "@radix-ui/themes"; interface SignalReportSummaryMarkdownProps { @@ -10,6 +13,14 @@ interface SignalReportSummaryMarkdownProps { variant: "list" | "detail"; /** Render in italic to indicate the summary is still being written. */ pending?: boolean; + /** + * Charts attached to the report. Detail only: the summary places a chart where + * it references one with `[label](chart:)`, and any the summary never + * references render after the prose. Omit to render markdown alone. + */ + charts?: ReportChart[]; + /** Required alongside `charts` — chart queries and links are report-scoped. */ + reportId?: string; } /** @@ -23,6 +34,8 @@ export function SignalReportSummaryMarkdown({ fallback, variant, pending, + charts, + reportId, }: SignalReportSummaryMarkdownProps) { const rawContent = content?.trim() ? content : fallback; const raw = formatSignalReportSummaryMarkdown(rawContent); @@ -32,6 +45,16 @@ export function SignalReportSummaryMarkdown({ const pendingClass = pending ? "italic" : ""; + // A single pass over the summary's lines, and detail views aren't hot — not + // worth memoising against the report's chart array identity. + const layout = + variant === "detail" && charts?.length + ? layoutReportSummary( + raw, + charts.map((chart) => chart.chart_id), + ) + : null; + if (variant === "list") { return ( - + {layout && charts && reportId ? ( + + ) : ( + + )} ); } + +/** + * Prose and charts interleaved in reading order, with any chart the summary + * never referenced appended below — an unreferenced chart is still evidence. + */ +function SummaryWithCharts({ + reportId, + charts, + layout, +}: { + reportId: string; + charts: ReportChart[]; + layout: ReturnType; +}) { + const placed = new Set(layout.placedChartIds); + const chartsById = new Map(charts.map((chart) => [chart.chart_id, chart])); + const trailing = charts.filter((chart) => !placed.has(chart.chart_id)); + + return ( +
+ {layout.segments.map((segment, index) => { + if (segment.kind === "markdown") { + return ( + // Segments are positional slices of one summary; order is their identity. + // biome-ignore lint/suspicious/noArrayIndexKey: segment order is the only identity +
+ +
+ ); + } + const chart = chartsById.get(segment.chartId); + if (!chart) return null; + return ( + + ); + })} + {trailing.map((chart) => ( + + ))} +
+ ); +} diff --git a/packages/ui/src/features/inbox/hooks/useReportChartData.ts b/packages/ui/src/features/inbox/hooks/useReportChartData.ts new file mode 100644 index 0000000000..f1e6050bf8 --- /dev/null +++ b/packages/ui/src/features/inbox/hooks/useReportChartData.ts @@ -0,0 +1,29 @@ +import { inboxReportKeys } from "@posthog/core/inbox/inboxQuery"; +import { reportChartQueryHash } from "@posthog/core/inbox/reportCharts"; +import type { ReportChart } from "@posthog/shared/types"; +import { useAuthStateValue } from "@posthog/ui/features/auth/store"; +import { useAuthenticatedQuery } from "@posthog/ui/hooks/useAuthenticatedQuery"; + +/** Chart results are a snapshot of evidence, not a live metric — refetch rarely. */ +const STALE_TIME_MS = 5 * 60 * 1000; + +/** + * Runs a report chart's query and hands back the raw `/query/` response for a + * shaper in `@posthog/core/inbox/reportCharts` to interpret. The chart's kind was + * validated when the report was normalized, so nothing unexpected reaches the + * query endpoint from here. + */ +export function useReportChartData(reportId: string, chart: ReportChart) { + const projectId = useAuthStateValue((state) => state.currentProjectId); + + return useAuthenticatedQuery>( + inboxReportKeys.chart( + projectId, + reportId, + chart.chart_id, + reportChartQueryHash(chart.query), + ), + (client) => client.runInsightQuery(chart.query), + { enabled: !!projectId, staleTime: STALE_TIME_MS, retry: false }, + ); +} diff --git a/packages/ui/src/utils/posthogLinks.ts b/packages/ui/src/utils/posthogLinks.ts index 1b2227fcdd..a202ef6030 100644 --- a/packages/ui/src/utils/posthogLinks.ts +++ b/packages/ui/src/utils/posthogLinks.ts @@ -74,6 +74,21 @@ export function featureFlagsIndexUrl(overrides?: LinkOverrides): string | null { return withProjectId((pid) => `/project/${pid}/feature_flags`, overrides); } +/** + * The report's page in the PostHog web app. Used to hand off evidence the + * desktop app can't draw itself — web renders report charts with the full + * insight stack. + */ +export function inboxReportUrl( + reportId: string, + overrides?: LinkOverrides, +): string | null { + return withProjectId( + (pid) => `/project/${pid}/inbox/${encodeURIComponent(reportId)}`, + overrides, + ); +} + export function skillUrl( skillName: string, overrides?: LinkOverrides,