Repository navigation
feat(experiments): queue a rerun for edits made during a recalculation - #93414
Conversation
|
😎 Merged successfully - details. |
🤖 CI report✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.
|
| Function | Location | Complexity | Limit |
|---|---|---|---|
pollRecalculation |
frontend/src/scenes/experiments/experimentMetricsLogic.ts:823 |
18 | 10 |
triggerRecalculation |
frontend/src/scenes/experiments/experimentMetricsLogic.ts:702 |
14 | 10 |
ExperimentReloadAction |
frontend/src/scenes/experiments/ExperimentView/ExperimentReloadAction.tsx:101 |
11 | 10 |
⚠️ Bundle size — 🔺 +482 B (+0.0%)
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 68.28 MiB · 🔺 +482 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.45 MiB · 22 files | no change | ███░░░░░░░ 32.3% of 4.51 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.89 MiB · 3,267 files | no change | █████████░ 91.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 |
| 854 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 307.3 KiB | ../node_modules/.pnpm/posthog-js@1.425.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js |
| 272.4 KiB | ../node_modules/.pnpm/posthog-js@1.425.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.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 |
| 255.2 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.5 KiB | src/lib/api.ts |
| 95.8 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.28 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.28 MiB · 18 files | no change | ████░░░░░░ 39.8% 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 |
|---|---|
| 754.1 KiB | dist/toolbar/toolbar-app-HCAVZOPH.css |
| 599.4 KiB | dist/toolbar/chunk-chunk-NXGDR7RI.js |
| 484.7 KiB | dist/toolbar/chunk-chunk-EL6RQSMS.js |
| 135.6 KiB | dist/toolbar/chunk-chunk-ED7KJGKB.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 71.3 KiB | dist/toolbar/toolbar-app-QDPL5Q45.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-67J3UW4R.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-WPC6J6SG.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 — 🔺 +9.9 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1458.69 MiB · 🔺 +9.9 KiB (+0.0%)
2ddcd86 to
722dd4c
Compare
🦔 Hogbox preview · ❌ build failedThe preview didn't come up for commit Previews are optional and never block merging. A failure here is often a hogland or tailnet hiccup rather than anything in your PR, so the check stays green and this comment is the status. |
rodrigoi
added
the
skip-inkeep-docs
Prompt To Fix All With AI### Issue 1
frontend/src/scenes/experiments/experimentMetricsLogic.ts:928-931
**Failed reruns lose queued edits**
When the follow-up create request fails, this branch has already cleared `queuedRerun`, while the failure handler only resets loading and displays a toast. The configuration edit is therefore forgotten and results remain stale until the user manually retries.
### Issue 2
frontend/src/scenes/experiments/experimentMetricsLogic.ts:928-932
**Polling exits strand queued reruns**
When polling reaches the 30-minute duration limit or five consecutive status failures, it exits without clearing the active recalculation or consuming `queuedRerun`. Because this terminal-response branch is the queue's only consumer, `isRecalculating` remains true and later edits accumulate in a rerun that never starts, leaving stale results indefinitely.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(experiments): simplifying the rules ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Proves the spec's headline edge case: two triggerRecalculation calls with different scopes during one live run coalesce to a single stronger-scope rerun that fires once on terminal.
… configuration change
…ever reaches terminal state
8aba65d to
5f7ba86
Compare
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved.
Frontend-only, in-memory queuing feature in a single owned area, authored by a team member with full historical ownership of these lines; the diff shows the retry-cap and stuck-poll edge cases flagged by the automated reviewer were already patched to clear the queued rerun, so no showstopper remains.
- Author wrote 100% of the modified lines and has 153 merged PRs in these paths (familiarity STRONG).
- 👍 on the PR from greptile-apps[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 52L, 3F substantive, 229L/4F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (229L, 4F, single-area, feat) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ 5f7ba86 · reviewed head 5f7ba86 |
Problem
When a user changes experiment or metric configuration while a metrics recalculation is running, the edit is lost. The backend enforces one active recalculation per experiment, so a mid-run change neither starts a fresh run nor is remembered for later. The user has to notice the run finished and reload by hand to apply the change.
Changes
This PR makes a config change made during a running recalculation start a follow-up run automatically once the current run finishes, without ever creating a second active run. It is frontend only; the backend "one active run per experiment" rule is untouched.
Queue the rerun in the metrics logic
experimentMetricsLogicgains a single in-memoryqueuedRerunitem. WhentriggerRecalculationfires with a config-change trigger while a run is active (isRecalculating), it records that trigger instead of posting a create. Only the two config-change triggers reach the queue; acold_runand any other trigger start their run normally. More changes during the same run coalesce onto that one item, keeping the stronger scope: once the queued item isexperiment_config_change(advance the window, recompute all) it stays there, and ametric_config_change(reuse the window, cache-hit unchanged metrics) never downgrades it.When the current run reaches a terminal status, the poll clears the queued item and fires it as a fresh recalculation. By that point the run is terminal, so the fresh run starts rather than re-queuing.
The queue is per tab and in memory, so a reload during a run drops it. Durable, server-side scheduling is a planned follow-up, out of scope here.
frontend/src/scenes/experiments/experimentMetricsLogic.tsReload control hint
While a rerun is queued, the reload control shows "Changes will apply after the current recalculation finishes", so the mid-run edit does not read as ignored. The button is already disabled during a run, so there is no double-submission.
frontend/src/scenes/experiments/ExperimentView/ExperimentReloadAction.tsxfrontend/src/scenes/experiments/ExperimentView/ExperimentReloadActionContainer.tsxHow did you test this code?
New logic tests cover the regressions a reviewer would worry about: a change during a run queues instead of posting a second run; a
cold_runstill starts fresh; multiple different-scope changes during one run coalesce to a single stronger-scope rerun that fires exactly once on terminal; and the fired rerun starts a fresh run rather than re-queuing.The reload-control hint is not yet verified in the running app; screenshot to follow.
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Model: Opus 4.8
Manually refactored: yes
Skills used:
Relevant decisions:
experimentMetricsLogic, so the backend "one active run per experiment" invariant and its DB constraint stay untouched. Durable server-side scheduling (aQUEUEDstatus) is a deliberate future stack.experiment_config_changeandmetric_config_change, so the reducer keepsexperiment_config_changeonce set and otherwise takes the incoming trigger. This replaced an earlier ranked-trigger helper, since only those two values ever reach the queue.setCurrentRecalculationflips the run terminal, soisRecalculatingis false and the rerun starts a fresh run instead of re-queuing.