fix(insights): compute on cache miss when loading an insight with dashboard overrides - #75858
Conversation
…hboard overrides Dashboard filter/variable overrides merge into the query before it is fingerprinted, so the overridden variant has its own cache key that no scheduled refresh warms. The insight scene loaded with refresh=async, which returns result: null on that cold key, and in dashboard context the data node never loads on its own — the scene dead-ends on "Chart data didn't load". Send refresh=async_except_on_cache_miss when overrides are present so a genuine miss computes and returns results. Also capture "insight refresh hint shown" when the dead-end state renders (it emitted nothing before), and rewrite the overrides banner on the insight scene in user-facing language. Generated-By: PostHog Code Task-Id: e5aeeeba-8a02-4465-bf6e-2a9bbfba3dbc
|
😎 This pull request was merged. |
🤖 CI report
|
| File | Size | Δ vs base |
|---|---|---|
render-query/src/render-query/render-query.js |
21.30 MiB | 🔺 +16.3 KiB (+0.1%) |
exporter/src/queries/schema.js |
1.06 MiB | 🔺 +14.9 KiB (+1.4%) |
posthog-app/src/queries/schema.js |
1.06 MiB | 🔺 +14.9 KiB (+1.4%) |
posthog-app/src/scenes/inbox/InboxScene.js |
711.9 KiB | 🔺 +13.5 KiB (+1.9%) |
posthog-app/src/scenes/instance/QueryPerformance/QueryPerformance.js |
34.7 KiB | 🔺 +1.6 KiB (+4.9%) |
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 | 🔺 +99 B (+0.0%) | ███░░░░░░░ 27.7% of 4.51 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.12 MiB · 3,023 files | 🔺 +1.6 KiB (+0.0%) | ████████░░ 83.6% 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.2/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 |
| 231.4 KiB | ../node_modules/.pnpm/posthog-js@1.409.2/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 | 🔺 +714 B (+0.0%) | ████░░░░░░ 38.3% of 5.72 MiB |
| Deferred (lazy) | 2.08 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.8 KiB | dist/toolbar/toolbar-app-2QFKEBT7.css |
| 551.1 KiB | dist/toolbar/chunk-chunk-5FDJUV5A.js |
| 484.6 KiB | dist/toolbar/chunk-chunk-QCKKTYHY.js |
| 133.6 KiB | dist/toolbar/chunk-chunk-UDBJ4EGL.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-T5KY5WYR.js |
| 71.0 KiB | dist/toolbar/toolbar-app-DBANC7W5.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-27JL52RE.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-QFZQEUXB.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-HWIVFWBZ.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 — 🔺 +332.9 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1371.02 MiB · 🔺 +332.9 KiB (+0.0%)
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
|
Reviews (1): Last reviewed commit: "fix(insights): compute on cache miss whe..." | Re-trigger Greptile |
There was a problem hiding this comment.
Contained frontend fix within the owning team's area, using an existing refresh mode rather than adding new API surface; diff matches the description, includes targeted tests, and has no security/data/CI implications, so no independent assurance is required.
- Author wrote 0% of the modified lines and has 106 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 63L, 4F substantive, 127L/5F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (127L, 5F, single-area, fix) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ abef3d7 · reviewed head 28ceb8a |
The E2E spec asserted the old banner phrase "filter/variable overrides", which this PR rewrote. Assert the new phrase instead. Generated-By: PostHog Code Task-Id: e5aeeeba-8a02-4465-bf6e-2a9bbfba3dbc
|
Retaining stamphog approval — delta since last review classified as |
TL;DR
Opening an insight from a dashboard applies the dashboard's filters, which gives the query its own cache key that nothing ever warms. The page could then show "Chart data didn't load" forever. Now that load asks the backend to compute when the cache misses, so the chart renders. The blank state also emits an analytics event (it was invisible before), and the overrides banner now reads like it's meant for users.
Problem
An insight opened with dashboard filter/variable overrides (e.g. clicking through from a dashboard tile) can dead-end on "Chart data didn't load", with manual Refresh or "Discard overrides" as the only ways out. Traced in this inbox report from session replays; the state emits no telemetry, so it is invisible in product analytics.
The chain:
refresh=async. On a cold key that mode enqueues a background computation and returnsresult: null— the result lands in cache moments later, but nothing on the scene polls or reloads.cachedInsight, anddataNodeLogicskips auto-loading when cached results exist on a dashboard-context key.Replaces #75842, which escalated the execution mode in
refresh_policy.pyfor override-carrying requests with norefreshparam. Every in-app path sends one (dashboard load:force_cache; tile refresh:blocking; insight scene:async), so that change never touched the broken flow — and it escalated every compute surface, turning e.g. astream_tilesrequest or an/insights/?filters_override=…list into per-insight blocking computes for API callers.Changes
insightLogic.loadInsightsendsrefresh=async_except_on_cache_missinstead ofasyncwhen overrides are present: warm and stale keys behave exactly as before, a genuine miss computes synchronously and returns real results. Override presence reuses thehasOverridespredicate, extracted asinsightOverridesPresent, so empty override objects don't escalate.InsightRefreshDataHintcapturesinsight refresh hint shown(dashboard id, insight short id, override presence) whenever the dead-end state renders.How did you test this code?
Automated, run locally:
jest src/scenes/insights/insightLogic.test.ts— 52 passed. The 3 new cases pin the refresh param the scene load sends:asyncwithout overrides,async_except_on_cache_misswith a filter override, andasyncagain for an empty override object. No existing test covered the scene's refresh mode.oxfmt --checkandoxlint— clean on the touched files.pnpm --filter=@posthog/frontend typescript:check— no errors in the touched files (pre-existing failures in quill-consuming products in this sandbox, unrelated).Not done: no manual browser verification (no dev stack in this sandbox). To reproduce: open a dashboard tile's insight with a filter override whose combination has never been computed, and confirm the chart renders instead of "Chart data didn't load".
Automatic notifications
Docs update
No user-facing docs affected.
🤖 Agent context
Human-driven (agent-assisted): implemented from the inbox report linked above, after review of #75842 found its escalation only applied to requests no in-app surface makes.
Decisions along the way:
async_except_on_cache_missis an existing, first-class refresh mode, and the "explicit?refresh=wins" contract stays intact for every other surface and API caller.insight refresh hint shownevent is there to measure it before fixing it.Created with PostHog Code