Repository navigation
feat(experiments): reuse metric results when changing metric configuration - #90484
Conversation
rodrigoi
added
the
skip-inkeep-docs
|
😎 Merged successfully - details. |
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
🤖 CI report
|
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.37 MiB · 22 files | no change | ███░░░░░░░ 30.3% of 4.51 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.81 MiB · 3,247 files | no change | █████████░ 90.7% 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 |
|---|---|
| 307.0 KiB | ../node_modules/.pnpm/posthog-js@1.422.0_@types+react@18.3.27_react@18.3.1/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 |
| 263.4 KiB | ../node_modules/.pnpm/posthog-js@1.422.0_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.js |
| 252.5 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 154.2 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 |
| 104.7 KiB | src/lib/api.ts |
| 95.2 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.26 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.26 MiB · 18 files | no change | ████░░░░░░ 39.5% of 5.72 MiB |
| Deferred (lazy) | 2.11 MiB · 45 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 |
|---|---|
| 748.9 KiB | dist/toolbar/toolbar-app-XQVFORZE.css |
| 588.0 KiB | dist/toolbar/chunk-chunk-OXL6LXNO.js |
| 484.7 KiB | dist/toolbar/chunk-chunk-QMGLBVDO.js |
| 133.8 KiB | dist/toolbar/chunk-chunk-SG2IDZGT.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 71.3 KiB | dist/toolbar/toolbar-app-G3JFVKNY.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-HETVH4RC.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-F63HJZZE.js |
| 6.8 KiB | dist/toolbar/chunk-chunk-DV7IWQNF.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +7.2 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1434.44 MiB · 🔺 +7.2 KiB (+0.0%)
⚠️ Playwright — 1 failed
🎭 Playwright report · View test results →
❌ 1 failed test:
- Hover chart to see tooltip with data point values (chromium)
These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!
⚠️ Django migration SQL — 1 new migration to review
We've detected new migrations on this PR. Review the SQL output for each migration:
products/experiments/backend/migrations/0036_alter_experimentmetricsrecalculation_trigger.py
BEGIN;
--
-- Alter field trigger on experimentmetricsrecalculation
--
-- (no-op)
COMMIT;Last updated: 2026-08-27 21:15 UTC (fcb5365)
✅ Django migration risk — migration analysis complete
We've analyzed your migrations for potential risks.
Summary: 0 Safe | 1 Needs Review | 0 Blocked
⚠️ Needs Review
May have performance impact
experiments.0036_alter_experimentmetricsrecalculation_trigger
└─ #1 ⚠️ AlterField
Field alteration may cause table locks or data loss (check if changing type or constraints)
model: experimentmetricsrecalculation, field: trigger, field_type: CharField
Last updated: 2026-08-27 21:15 UTC (fcb5365)
|
Reviews (1): Last reviewed commit: "feat(experiments): reuse metric results ..." | Re-trigger Greptile |
🦔 PostHog Review reviewed this pull requestFound 3 must fix, 2 should fix, 2 consider. Published 7 findings (view the review). Resolved comments: 3 declined, 2 already settled |
|
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
25bb8a6 to
fcb5365
Compare
Problem
We used to reload all the experiment results when the user changed the experiment configuration or any metric. On experiments with multiple metrics, we relied on the query runner cache rules to reduce server load. We are changing that default behavior so some configuration changes apply only to the intended metrics.
Changes
We are splitting the
config_changetrigger into experiment configuration changes and metric configuration changes. When a metric has a configuration change (for example, adding a breakdown), the backend will reuse the last completed runquery_to, so all unchanged metrics will return from the results cache, while changed metrics will recalculate and return new values.We still run the workflow activities for each metric, but that's an optimization for a different time.
How did you test this code?
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Automatic notifications
Docs update
🤖 Agent context
No agent involved.