fix(insights): auto-wrap bare query sources saved via the REST API - #73333
Conversation
Insights created via the API with a bare query source (e.g. TrendsQuery) saved and executed fine, but rendered as blank tiles in the UI, which only routes wrapper nodes (InsightVizNode, DataVisualizationNode) to chart renderers. The MCP path already normalized these via MCPInsightSerializer, gated on the x-posthog-client header; every other API caller slipped through unwrapped. Promote the auto-wrapping to the base InsightSerializer: bare Trends/Funnels/Retention/Paths/Stickiness/Lifecycle queries get wrapped in InsightVizNode, bare HogQLQuery in DataVisualizationNode. Everything else is stored verbatim, so no query accepted today starts failing, wrapped frontend saves round-trip untouched, and kinds the UI renders bare (e.g. WebOverviewQuery) keep their shape. Hard rejection of unsavable queries stays MCP-only. Generated-By: PostHog Code Task-Id: b2fec550-7b08-4a6f-8674-b7a3c2c0f4c3
|
Too many files changed for review. ( Bypass the limit by tagging |
The auto-commit from the OpenAPI bot rewrote every generated MCP file with a different toolchain, adding ~70k lines of churn that has nothing to do with this change (the schema is unchanged). Reset all generated files to match master so the PR carries only the two real source files. Generated-By: PostHog Code Task-Id: af8f9559-d044-4311-9ede-e183bf832675
Generated-By: PostHog Code Task-Id: af8f9559-d044-4311-9ede-e183bf832675
Generated-By: PostHog Code Task-Id: 9c91bf54-7f49-4718-842f-b82daf43f14c
🤖 CI report
|
| File | Patch | Uncovered changed lines |
|---|---|---|
products/product_analytics/backend/api/insight.py |
76.9% | 742, 752–753 |
🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 30073781147 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
platform_features |
██░░░░░░░░░░░░░░░░░░ 12.1% |
7 / 58 |
batch_exports |
████████░░░░░░░░░░░░ 39.6% |
8,414 / 21,249 |
demo |
███████████░░░░░░░░░ 56.2% |
1,497 / 2,663 |
warehouse_sources_queue |
████████████░░░░░░░░ 59.2% |
148 / 250 |
tasks |
██████████████░░░░░░ 69.4% |
30,000 / 43,235 |
data_tools |
██████████████░░░░░░ 70.0% |
63 / 90 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
signals |
████████████████░░░░ 79.9% |
21,906 / 27,415 |
cdp |
████████████████░░░░ 81.0% |
3,144 / 3,881 |
data_modeling |
█████████████████░░░ 82.7% |
5,547 / 6,708 |
wizard |
█████████████████░░░ 84.8% |
935 / 1,102 |
notebooks |
█████████████████░░░ 85.3% |
7,275 / 8,531 |
agent_platform |
█████████████████░░░ 86.4% |
3,807 / 4,405 |
actions |
█████████████████░░░ 86.6% |
717 / 828 |
cohorts |
██████████████████░░ 87.8% |
4,488 / 5,114 |
product_tours |
██████████████████░░ 87.9% |
1,303 / 1,482 |
exports |
██████████████████░░ 88.4% |
6,949 / 7,861 |
data_warehouse |
██████████████████░░ 88.9% |
11,887 / 13,378 |
business_knowledge |
██████████████████░░ 89.0% |
4,391 / 4,936 |
conversations |
██████████████████░░ 89.3% |
16,834 / 18,848 |
engineering_analytics |
██████████████████░░ 89.4% |
6,386 / 7,145 |
dashboards |
██████████████████░░ 89.4% |
5,983 / 6,693 |
visual_review |
██████████████████░░ 89.4% |
5,818 / 6,505 |
error_tracking |
██████████████████░░ 89.7% |
10,174 / 11,342 |
alerts |
██████████████████░░ 90.0% |
4,056 / 4,508 |
early_access_features |
██████████████████░░ 90.1% |
1,031 / 1,144 |
mcp_analytics |
██████████████████░░ 90.1% |
2,763 / 3,065 |
streamlit_apps |
██████████████████░░ 90.4% |
2,501 / 2,767 |
links |
██████████████████░░ 90.6% |
183 / 202 |
slack_app |
██████████████████░░ 90.7% |
9,028 / 9,951 |
marketing_analytics |
██████████████████░░ 91.0% |
11,792 / 12,964 |
stamphog |
██████████████████░░ 91.1% |
4,056 / 4,450 |
mcp_store |
██████████████████░░ 91.9% |
4,257 / 4,634 |
notifications |
███████████████████░ 92.6% |
1,011 / 1,092 |
product_analytics |
███████████████████░ 92.7% |
5,840 / 6,300 |
ai_observability |
███████████████████░ 92.7% |
14,995 / 16,168 |
surveys |
███████████████████░ 93.0% |
5,730 / 6,163 |
managed_migrations |
███████████████████░ 93.1% |
1,424 / 1,530 |
web_analytics |
███████████████████░ 93.1% |
14,398 / 15,461 |
posthog_ai |
███████████████████░ 93.2% |
1,326 / 1,422 |
approvals |
███████████████████░ 93.3% |
3,437 / 3,682 |
reminders |
███████████████████░ 93.4% |
468 / 501 |
workflows |
███████████████████░ 93.6% |
6,390 / 6,826 |
legal_documents |
███████████████████░ 94.1% |
1,568 / 1,667 |
endpoints |
███████████████████░ 94.1% |
8,640 / 9,177 |
messaging |
███████████████████░ 94.3% |
2,682 / 2,845 |
tracing |
███████████████████░ 94.5% |
2,670 / 2,826 |
revenue_analytics |
███████████████████░ 94.6% |
3,651 / 3,858 |
review_hog |
███████████████████░ 94.7% |
6,814 / 7,199 |
skills |
███████████████████░ 94.7% |
3,086 / 3,258 |
logs |
███████████████████░ 95.4% |
9,935 / 10,416 |
experiments |
███████████████████░ 95.7% |
25,036 / 26,159 |
growth |
███████████████████░ 96.1% |
3,245 / 3,376 |
replay_vision |
███████████████████░ 96.2% |
15,170 / 15,774 |
annotations |
███████████████████░ 96.2% |
732 / 761 |
feature_flags |
███████████████████░ 96.5% |
17,110 / 17,734 |
user_interviews |
███████████████████░ 96.5% |
2,638 / 2,734 |
access_control |
███████████████████░ 96.8% |
851 / 879 |
customer_analytics |
███████████████████░ 97.1% |
9,047 / 9,317 |
warehouse_sources |
███████████████████░ 97.1% |
330,209 / 339,939 |
data_catalog |
███████████████████░ 97.5% |
2,368 / 2,429 |
analytics_platform |
████████████████████ 98.0% |
2,153 / 2,197 |
metrics |
████████████████████ 98.2% |
2,491 / 2,536 |
pulse |
████████████████████ 98.4% |
2,017 / 2,049 |
live_debugger |
████████████████████ 99.2% |
613 / 618 |
field_notes |
████████████████████ 99.4% |
158 / 159 |
Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.
There was a problem hiding this comment.
The substantive change is a narrow, well-tested serializer fix (42 lines in one file) authored by a member of the owning team for that file; the remaining 560+ lines across 57 files are purely mechanical import-reordering in auto-generated MCP files with no logic change.
- Author wrote 1% of the modified lines and has 17 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from hex-security-app[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 42L, 1F substantive, 602L/58F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (602L, 58F, two-areas, fix) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ db92fc4 · reviewed head 256bc93 |
TL;DR
If you saved an insight through the API with a "bare" query like
{"kind": "TrendsQuery", ...}, PostHog stored it and could run it, but the UI showed an empty tile because it only knows how to draw queries wrapped in{"kind": "InsightVizNode", "source": ...}. Now the API adds that wrapper for you on save, so these insights render.Problem
Insights created via the REST API with a bare query source (e.g.
TrendsQuery, or a rawHogQLQuery) save and execute fine, but render as blank tiles in the UI.Query.tsxonly routes wrapper nodes (InsightVizNode,DataVisualizationNode,DataTableNode, ...) to chart renderers; a bare source falls through to a JSON-dump fallback that measures ~0px inside a dashboard tile.The fix for this already exists:
MCPInsightSerializer.validate_querynormalizes bare queries into the right wrapper, but only when the request carries thex-posthog-client: mcpheader. Every other API caller (scripts, agents, integrations) slips through unwrapped. This came in as customer feedback (Slack thread, ticket #2916) after it cost a customer real debugging time.Changes
Promote the auto-wrapping (and only the auto-wrapping) from the MCP serializer to the base
InsightSerializer, so it applies to all create and update paths:TrendsQuery/FunnelsQuery/RetentionQuery/PathsQuery/StickinessQuery/LifecycleQueryget wrapped in anInsightVizNode.HogQLQuerygets wrapped in aDataVisualizationNode.Note
Deliberately narrower than the MCP path, to keep the blast radius near zero:
QuerySchemaParseralready rejects schema-invalid queries at parse time, unchanged).InsightVizNode.sourceunion. The union also acceptsWebOverviewQuery, but the UI renders that kind bare (web analytics tiles), so wrapping it would change how those render. The six classic insight kinds only render insideInsightVizNode.MCPInsightSerializerbehavior is unchanged (only itsvalidate_querysignature widened to match the base override for mypy).How did you test this code?
Ran locally (all green):
products/product_analytics/backend/api/test/test_insight_query.py(19 passed, includes the 6 new cases and the pre-existing MCP normalization tests)products/product_analytics/backend/api/test/test_insight.py(173 passed, 3 skipped)posthog/api/test/test_alert_real_time_interval.py,posthog/api/test/test_sharing_access_token_security.py(both create insights via the API)uv run mypy --cache-fine-grained .(clean)New tests and the regression each catches, which no existing test did (existing normalization tests only cover the MCP header path):
test_create_wraps_bare_query_sources: the REST path stops wrapping bare queries, bringing blank tiles back.test_create_stores_non_bare_queries_verbatim: someone routes wrapped or unrecognized queries through pydantic re-dump or promotes the MCP hard rejection to the base serializer, silently rewriting frontend saves or 400ing previously accepted queries. Includes the bareWebOverviewQuerycase to pin the deliberate allowlist exclusion.test_update_wraps_bare_query_sources: normalization gets wired into create only, leaving updates broken.Automatic notifications
Docs update
No docs under
docs/reference the bare-query pitfall or the MCP-only normalization; API reference docs are generated from the OpenAPI schema, which is unchanged (behavior only).🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored by PostHog Code (Claude) from a Slack bug report handed to Thomas. Skills invoked:
/improving-drf-endpoints,/writing-tests.Decisions along the way:
validate_query(wrap + reject) into the base serializer, as the customer feedback suggested, but rejected it: it would 400 queries the API accepts today and silently rewrite wrapped queries on the main frontend save path.InsightVizNode.sourceunion instead of an allowlist, butWebOverviewQueryis in the union and renders bare in the UI, so wrapping it would change existing behavior. The allowlist fails closed for future kinds.hogli build:openapilocally produced ~69k lines of churn across 116 generated files even with no schema-affecting change, so I left generated files untouched; the schema is unchanged and CI's openapi job auto-commits real drift with the canonical toolchain.Created with PostHog Code