Skip to content

fix(insights): compute insights when dashboard overrides miss the cache - #75842

Closed
posthog[bot] wants to merge 1 commit into
masterfrom
posthog-code/compute-insights-under-overrides
Closed

fix(insights): compute insights when dashboard overrides miss the cache#75842
posthog[bot] wants to merge 1 commit into
masterfrom
posthog-code/compute-insights-under-overrides

Conversation

@posthog

@posthog posthog Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Problem

  • Anyone viewing a dashboard with filters, variables, or tile overrides applied can get blank charts reading "Chart data didn't load" instead of results, with a manual Refresh as the only way out.
  • Overrides get merged into the query before it is fingerprinted, so the overridden variant has a cache key of its own that no scheduled refresh ever warms.
  • Every surface defaults to CACHE_ONLY_NEVER_CALCULATE when the client sends no ?refresh=, so that guaranteed-cold key is read cache-only, returns result: null, and the UI renders the dead-end tile.
  • Opening the insight from that dashboard doesn't help: it still counts as dashboard context, and the frontend force-refresh that exists for exactly this case bails because there's no cachedInsight on that route.

Changes

  • An override-carrying request no longer falls back to cache-only. It resolves to RECENT_CACHE_CALCULATE_ASYNC_IF_STALE_AND_BLOCKING_ON_MISS: serve the cache when warm, refresh in the background when stale, compute synchronously only on a genuine miss.
Request Before After
No ?refresh=, no overrides cache-only cache-only (unchanged)
No ?refresh=, overrides present cache-only → result: null async if stale, blocking on miss
Explicit ?refresh=force_cache + overrides cache-only cache-only (opt-out still wins)
Shared / embedded clamped clamped (unchanged - overrides are ignored there)
  • InsightRefreshDataHint now captures insight refresh hint shown with dashboard id, insight short id, and whether overrides were active. The hint emitted nothing before, which is why this only ever surfaced through session replay.
  • Rewrote the overrides banner on the insight scene, which read like an internal note.
-You are viewing this insight with filter/variable overrides. Discard them to edit the insight.
+You're viewing this insight with a dashboard's filters applied, so you can't edit it. Discard them to go back to the saved insight.

Note

This shifts some override-carrying requests from a free cache read to a blocking compute on miss. It is bounded to requests that send no ?refresh= at all, and the dashboard tile loader already sends an explicit refresh, so the main callers picking this up are the insight scene and API clients that were getting null anyway.

How did you test this code?

Automated, run locally:

  • posthog/hogql_queries/test/test_refresh_policy.py (20 passed). New cases: each of the three override params escalates past cache-only on every surface (the regression above, which no existing case covered since the parity snapshot only exercises override-free requests); an explicit ?refresh=force_cache with overrides stays cache-only; the shared path resolves identically with and without overrides.
  • frontend/src/queries/nodes/InsightViz/InsightVizDisplay.test.tsx (57 passed).
  • pnpm --filter=@posthog/frontend typescript:check - clean for the touched files (unrelated pre-existing errors in quill-consuming products in this sandbox).

Not done: no manual or visual verification. This sandbox has no Docker, so the dev stack, DB-backed API tests, and a browser repro of the blank tile were all out of reach. A reviewer running locally can reproduce by opening a dashboard tile with a filter override that has never been computed and confirming the chart renders instead of the refresh hint.

Automatic notifications

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

Docs update

No user-facing docs affected.

🤖 Agent context

Autonomy: Fully autonomous

Written by Claude (Claude Code) from a PostHog inbox report. Skills invoked: /writing-tests.

Decisions along the way:

  • Escalating in refresh_policy.py rather than inside insight_result keeps the per-surface default table the single place refresh behavior is decided, and keeps the precedence rule (explicit ?refresh= wins) in one function.
  • Chose blocking-on-miss over plain async-on-miss so the response carries real results, which every existing client handles. Plain async would have depended on each caller polling query_status, which the insight scene path does not reliably do.
  • Considered but skipped extending the insightDataLogic afterMount force-refresh to the insight scene. Once the backend computes, both observed cases are covered, and adding a second load trigger on a route that already loads risks duplicate queries. Worth revisiting if the backend fix turns out not to cover a case.
  • Gated the escalation on the client-supplied query params, not on the merged filters, since the override helpers fall back to the dashboard's own saved filters, and those keys are warmed by the scheduled refresh.

Created with PostHog Desktop from this inbox report.

Requests carrying filters_override, variables_override or tile_filters_override changed the
cache key but were still read cache-only, so the override-specific entry no scheduled refresh
warms returned result: null and the UI rendered a dead-end "Chart data didn't load" tile.

Such requests now fall back to async-if-stale, blocking-on-miss unless the client sent an
explicit ?refresh= or the request is shared/embedded (where overrides are ignored anyway).

Also adds telemetry to the refresh hint so the failure stops being invisible, and rewrites the
overrides banner on the insight scene.

Generated-By: PostHog Code
Task-Id: e3e48876-88f0-45b0-8636-27a1b22a3ff9
@trunk-io

trunk-io Bot commented Jul 31, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Bundle size — 🔺 +423 B (+0.0%)

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

Total: 65.35 MiB · 🔺 +423 B (+0.0%)

No file changed by more than 1000 B.

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.25 MiB · 22 files no change ███░░░░░░░ 27.7% of 4.51 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
8.11 MiB · 3,021 files no change ████████░░ 83.5% of 9.71 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
762 B src/index.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
285.3 KiB ../node_modules/.pnpm/posthog-js@1.409.0/node_modules/posthog-js/dist/rrweb.js
267.7 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
235.5 KiB src/taxonomy/core-filter-definitions-by-group.json
226.9 KiB ../node_modules/.pnpm/posthog-js@1.409.0/node_modules/posthog-js/dist/module.js
154.3 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.2 KiB src/lib/api.ts
94.7 KiB ../packages/quill/packages/quill/dist/index.js
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
90.6 KiB ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/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

Toolbar bundle — eager 2.19 MiB within budget

What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.

Metric Size Δ vs base Budget
Eager (shipped)
entry + static imports
2.19 MiB · 17 files no change ████░░░░░░ 38.2% of 5.72 MiB
Deferred (lazy) 2.07 MiB · 33 files no change n/a — loads on demand
Loader dist/toolbar.js 1.1 KiB no change █░░░░░░░░░ 5.8% of 19.5 KiB
Largest eagerly-shipped chunks
Size File
717.7 KiB dist/toolbar/toolbar-app-NOGOCMN4.css
546.6 KiB dist/toolbar/chunk-chunk-45CC4NVD.js
484.4 KiB dist/toolbar/chunk-chunk-J6LEKXMR.js
133.6 KiB dist/toolbar/chunk-chunk-INYNBXIO.js
131.8 KiB dist/toolbar/chunk-chunk-T5KY5WYR.js
71.0 KiB dist/toolbar/toolbar-app-IXFB6SXZ.js
69.0 KiB dist/toolbar/chunk-chunk-27JL52RE.js
35.6 KiB dist/toolbar/chunk-chunk-W55RZ5CW.js
20.9 KiB dist/toolbar/chunk-chunk-BTBOG7BH.js
12.2 KiB dist/toolbar/chunk-chunk-PIK3PADE.js

Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile

Dist folder size — 🔺 +6.3 KiB (+0.0%)

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

Total: 1370.56 MiB · 🔺 +6.3 KiB (+0.0%)

Copy link
Copy Markdown
Collaborator

Closing in favor of #75858.

Review of this change found the escalation in refresh_policy.py only applies when a request carries overrides but no refresh param — and every in-app surface behind the linked report sends one (dashboard load: force_cache; per-tile refresh: blocking; insight scene: async). So the recorded failures were unchanged, while the escalation did apply to every compute surface for API callers, e.g. turning a stream_tiles request or an /insights/?filters_override=… list into per-insight blocking computes.

#75858 fixes the broken caller instead: the insight scene sends refresh=async_except_on_cache_miss when overrides are present, and it keeps the hint telemetry and banner rewrite from here.

This comment was posted by PostHog Code on behalf of Thomas Obermueller.

@thmsobrmlr thmsobrmlr closed this Jul 31, 2026
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.

1 participant