Skip to content

feat(metrics): render MetricsQuery insights and save from the viewer - #69446

Merged
DanielVisca merged 4 commits into
posthog-code/metrics-query-nodefrom
posthog-code/metrics-insights-frontend
Jul 9, 2026
Merged

feat(metrics): render MetricsQuery insights and save from the viewer#69446
DanielVisca merged 4 commits into
posthog-code/metrics-query-nodefrom
posthog-code/metrics-insights-frontend

Conversation

@DanielVisca

Copy link
Copy Markdown
Contributor

Problem

Stacked on #69445, which adds the MetricsQuery node kind and backend runner. Without a Query.tsx renderer the node cannot appear on insights or dashboard tiles, and without a save path nothing produces MetricsQuery insights in the first place.

Changes

  • MetricsQueryNode (products/metrics/frontend/nodes/) mounts dataNodeLogic and renders the new shared MetricsSeriesChart, dispatched from Query.tsx via an isMetricsQuery guard, so saved metrics insights render on the insight scene, dashboard tiles, and notebooks.
  • MetricsSeriesChart is extracted from the metrics viewer (Sparkline + legend + the time-axis ladder) and the viewer now uses it too, so tiles and the viewer render identically and cannot drift.
  • "Save as insight" in the viewer persists the current viewer state as a MetricsQuery insight (loader-backed button, disabled without a metric, toast links to the saved insight). The viewer's p95 shorthand maps to the node's quantile aggregation.

Screenshots to follow after a local stack run; the viewer chart itself is unchanged visually (same components, extracted).

How did you test this code?

  • hogli test products/metrics/frontend/components/metricsViewerLogic.test.ts — 7 passed; the 2 new cases lock the viewer-state → MetricsQuery mapping (a wrong mapping silently saves insights that re-run a different query than the viewer showed) and the no-metric null case.
  • hogli test products/metrics/backend/tests/ — 159 passed, including the insight-persistence gate added in this branch.
  • Frontend typecheck: no errors in touched files (pre-existing unrelated failures in other products' stale generated types).
  • I (the agent) did not run a browser against the dashboard tile; API-level save + query execution are test-covered.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Docs update

Covered by the metrics doc-sync PRs (#69407, #69409).

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Claude Code (Fable 5) followed the RevenueAnalytics node-component pattern for the Query.tsx integration and the SQL editor's insightsApi.create pattern for saving. Skills invoked: /writing-tests, /pr-slicing, /get-stamp. Decisions: extract the chart into a shared component rather than duplicate the axis ladder in the tile renderer, and keep the save flow modal-free (default name metric (aggregation), rename on the insight page) to ship the end-to-end path first.

@DanielVisca DanielVisca self-assigned this Jul 8, 2026
@DanielVisca
DanielVisca marked this pull request as ready for review July 8, 2026 18:47
@github-actions
github-actions Bot requested a deployment to preview-pr-69446 July 8, 2026 18:47 In progress
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🦔 Hogbox preview · ❌ build failed

The preview didn't come up for commit 599e47c. See the build log for the failing step. It'll retry on the next push.

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested review from a team July 8, 2026 18:47
Comment thread products/metrics/frontend/components/metricsViewerLogic.tsx
@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "chore(metrics): lock insight persistence..." | Re-trigger Greptile

Comment thread products/metrics/frontend/components/metricsViewerLogic.tsx
@posthog

posthog Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

👋 Visual changes detected for this PR.

Review and approve in PostHog Visual Review

If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix.

@trunk-io

trunk-io Bot commented Jul 8, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Bundle size — 🔺 +4.0 KiB (+0.0%)

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 69.89 MiB · 🔺 +4.0 KiB (+0.0%)

File Size Δ vs base
render-query/src/render-query/render-query.js 24.11 MiB 🔺 +3.0 KiB (+0.0%)

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.20 MiB · 22 files no change ███░░░░░░░ 28.0% of 4.29 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
8.07 MiB · 2,956 files 🔺 +56 B (+0.0%) █████████░ 87.2% of 9.25 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/RootErrorBoundary.tsx
912 B ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js
789 B src/scenes/ChunkLoadErrorBoundary.tsx
668 B src/index.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
277.1 KiB ../node_modules/.pnpm/posthog-js@1.398.2/node_modules/posthog-js/dist/rrweb.js
266.9 KiB ../node_modules/.pnpm/@posthog+icons@0.37.4_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
224.6 KiB src/taxonomy/core-filter-definitions-by-group.json
213.6 KiB ../node_modules/.pnpm/posthog-js@1.398.2/node_modules/posthog-js/dist/module.js
164.0 KiB src/queries/validators.js
154.0 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
105.7 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
90.5 KiB ../node_modules/.pnpm/@tiptap+core@3.20.1_@tiptap+pm@3.20.1/node_modules/@tiptap/core/dist/index.js

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

⚠️ Dist folder size — 🔺 +55.6 KiB (+0.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 1433.84 MiB · 🔺 +55.6 KiB (+0.0%)

Playwright — all passed

All tests passed.

View test results →

@DanielVisca DanielVisca added the stamphog Request AI approval (no full review) label Jul 8, 2026
@stamphog

stamphog Bot commented Jul 8, 2026

Copy link
Copy Markdown

Note

🤖 stamphog reviewed dc8102aecf618d26e28a1fad94a4435efc7c0888 — verdict: REFUSED

The diff imports/depends on symbols and files (NodeKind.MetricsQuery, MetricsQuery/MetricsQueryClause/MetricsQueryFilter/MetricsQueryResponse, and products/metrics/backend/tests/test_metrics_query_runner.py with class TestMetricsQueryRunner) that do not exist anywhere in the reviewed checkout — I grepped schema-general.ts, posthog/schema.py, query_runner.py, and the actual metrics backend test files and found only the pre-existing singular MetricQueryRunner/MetricsQueryAPI, never the plural MetricsQuery node kind this PR builds on. The stacked base PR (#69445) this branch is said to sit on is not present in the tree being reviewed.

  • Author wrote 100% of the modified lines and has 20 merged PRs in these paths (familiarity STRONG).
  • DanielVisca reviewed the current head.
  • Diff cannot be verified to apply or compile against the reviewed checkout — it edits products/metrics/backend/tests/test_metrics_query_runner.py and class TestMetricsQueryRunner, neither of which exists there (only the unrelated singular test_metric_query_runner.py / TestMetricQueryRunner does).
  • NodeKind.MetricsQuery and the MetricsQuery/MetricsQueryClause/MetricsQueryFilter/MetricsQueryResponse types the frontend code imports are absent from frontend/src/queries/schema/schema-general.ts and posthog/schema.py on this checkout.
  • No backend dispatch case for kind == "MetricsQuery" exists in posthog/hogql_queries/query_runner.py, and no products/metrics/backend/facade/queries.py registers a MetricsQueryRunner — so if this frontend save path ("Save as insight") were live, saved insights would raise Unsupported query kind on every dashboard render/refresh.
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 346L, 8F substantive, 394L/10F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1d-complex (394L, 10F, two-areas, feat)
stamphog 2.0.0b3 .stamphog/policy.yml @ 466db01 · reviewed head dc8102a

Updated in place — this replaces 1 earlier stamphog review(s) on this PR.

@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Jul 8, 2026
@DanielVisca

Copy link
Copy Markdown
Contributor Author

Re: the refusal reason — this is a stacked PR (base: posthog-code/metrics-query-node, PR #69445). The backend dispatch for MetricsQuery is not in this diff because it lands in the base PR: posthog/hogql_queries/query_runner.py registers MetricsQueryRunner (via products/metrics/backend/facade/queries.py) on that branch, so it is present in this PR's tree. It is exercised by this branch's own CI: test_generic_query_endpoint_accepts_metrics_query and test_insight_saves_with_metrics_query in products/metrics/backend/tests/test_metrics_query_runner.py both pass here. Rendering a saved MetricsQuery insight therefore cannot hit an unhandled-kind error on this branch. This PR cannot merge before #69445 by construction (GitHub base-branch ordering).

@DanielVisca DanielVisca added the stamphog Request AI approval (no full review) label Jul 8, 2026
@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Jul 8, 2026
@DanielVisca DanielVisca added the priority-review Posts to Slack. label Jul 8, 2026
@DanielVisca
DanielVisca force-pushed the posthog-code/metrics-insights-frontend branch from dc8102a to 6b60463 Compare July 8, 2026 21:01
@github-actions
github-actions Bot requested a deployment to preview-pr-69446 July 8, 2026 21:02 In progress
Comment thread products/metrics/frontend/components/metricsViewerLogic.tsx
DanielVisca and others added 3 commits July 8, 2026 15:04
…rom viewer

Adds the frontend half of the MetricsQuery node kind: a Query.tsx dispatch
branch rendering the shared MetricsSeriesChart (extracted from the viewer
so tiles and the viewer render identically), and a Save as insight action
in the metrics viewer that persists the current viewer state as a
MetricsQuery insight, addable to any dashboard. The viewer's p95 shorthand
maps to the node's quantile aggregation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Guards MetricsQuery's membership in the QuerySchema union: removing it
would make every saved metrics insight fail validation silently.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@DanielVisca

Copy link
Copy Markdown
Contributor Author

Review fix pushed (now at 44f515c after restacking on #69445):

  • Silent save failure fixed (greptile P1): a saveAsInsightFailure listener now surfaces insight-save errors as an error toast instead of the spinner silently disappearing; the success path was already toasting.
  • Restacked on the base PR's CI fixes (tach boundary, SavedInsights metadata, metrics scope enforcement). The remaining Playwright red is a master-wide outage (identical spec set fails on master); a rerun is queued for when master heals.

… into posthog-code/metrics-insights-frontend

# Conflicts:
#	products/metrics/frontend/components/MetricsViewer.tsx
#	products/metrics/frontend/components/metricsViewerLogic.test.ts
#	products/metrics/frontend/components/metricsViewerLogic.tsx
@DanielVisca
DanielVisca merged commit f22b0c8 into posthog-code/metrics-query-node Jul 9, 2026
159 of 162 checks passed
@DanielVisca
DanielVisca deleted the posthog-code/metrics-insights-frontend branch July 9, 2026 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants