Skip to content

fix(experiments): keep the experiment duration dates consistent - #89644

Draft
phillram wants to merge 1 commit into
masterfrom
posthog/fix-experiment-duration-date-consistency
Draft

fix(experiments): keep the experiment duration dates consistent#89644
phillram wants to merge 1 commit into
masterfrom
posthog/fix-experiment-duration-date-consistency

Conversation

@phillram

@phillram phillram commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Problem

A rejected date change stays on the experiment page while the experiments list and the metric results show the date the server stored, so one experiment reads as having two start dates. Only a toast reports the failure.

Date history is also unreadable. experiment start date changed reads the experiment after the update replaced it, so the old and new dates match on every event. Checked against a real experiment with 11 date changes: old equals new on all 11.

From: https://us.posthog.com/project/2/support/tickets/69196

Changes

  • A rejected date snaps back to what the server stored.
  • The date-change events report the previous date again.
  • The date buttons refuse clicks while a change saves, which frontend/src/AGENTS.md requires.
  • A failed date change no longer triggers a metrics recalculation, and no longer leaves an unhandled rejection.

Dropping the dates from the preserved-fields set is the judgment call. Preserving a rejected edit suits a half-typed name. A date is one click to re-pick, and holding a date nothing stored is worse than losing it.

reorderMetrics in this logic solves a nearby problem with an optimistic write plus a 300 ms debounce. It does not fit here: the observed edits sat 6 to 31 seconds apart, so no debounce window would coalesce them.

Note

Stored dates and computed metrics still disagree in three documented backend cases. The metric fingerprint derives from start_date, so a change mid-recalculation orphans that run's rows. The ClickHouse cache key does not vary with start_date. An API caller that omits version overwrites a concurrent edit.

Nothing looks different outside the in-flight spinner.

How did you test this code?

Four cases in experimentLogic.test.ts, each confirmed to fail with its fix reverted. Two catch the reported date being the new one, reproducing what the events show today. Two catch a rejected date staying in local state. The existing conflict test pins the opposite policy for description, so both halves are covered.

The button guard has no test. It passes one loading prop to a LemonButton, so a test would assert the prop rather than a behavior.

Not done: no manual run of the app. hogli is unavailable in this sandbox, so ci:preflight and hogli review did not run. oxlint, oxfmt, jest and tsgo --noEmit ran directly, with no errors in the changed files.

Automatic notifications

  • Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Claude Code (Opus 4.5), from a support question about an experiment reporting two different start dates.

Two theories died on the data. A stale cache or lost write: ruled out, because the update loader already serializes concurrent dispatches and carries a concurrency token. Missing save feedback causing re-clicks: ruled out once the edit intervals measured 6 to 31 seconds. An earlier draft of this description asserted the second theory and was cut. The reported symptom is most likely not a bug at all, since the screenshots are best explained by being taken hours apart across a date change, on an experiment backdated to the day before it was created.

Skills invoked: /writing-tests, /writing-pr-descriptions.

Public artifact: the investigation used a customer's usage data. No customer name, account, project, person, experiment name, or event figure appears anywhere in this PR. The tests use the existing repo fixture and invented dates.


Created with PostHog Desktop

The duration date buttons gave no feedback while a change saved, so the label kept
showing the old date and people re-picked. Each extra pick is another write and
another forced metrics recalculation.

Three changes:

- The date buttons show a loading state and refuse clicks while the change saves.
- A rejected date no longer stays on screen. The tab used to hold a date the server
  never stored while the list and the results showed the stored one.
- The start and end date telemetry reports the previous date again. It read the
  experiment after the update replaced it, so both sides of the change carried the
  new date and no record of the old one survived.

Generated-By: PostHog Desktop
Task-Id: ab8d1258-e929-46e2-9b30-ce03899d171f
@trunk-io

trunk-io Bot commented Aug 26, 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 Aug 26, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

Trunk lane — non-backend lane

This 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.

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

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

Total: 68.40 MiB · 🔺 +119 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.36 MiB · 22 files no change ███░░░░░░░ 30.3% of 4.51 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
8.78 MiB · 3,237 files 🔺 +24 B (+0.0%) █████████░ 90.4% 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
306.5 KiB ../node_modules/.pnpm/posthog-js@1.418.17/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
258.5 KiB ../node_modules/.pnpm/posthog-js@1.418.17/node_modules/posthog-js/dist/module.js
246.0 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.6 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.25 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.25 MiB · 17 files no change ████░░░░░░ 39.3% of 5.72 MiB
Deferred (lazy) 2.09 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
746.7 KiB dist/toolbar/toolbar-app-RBPKVAJT.css
582.8 KiB dist/toolbar/chunk-chunk-6GSV5CPA.js
484.6 KiB dist/toolbar/chunk-chunk-6S5AAQW2.js
133.8 KiB dist/toolbar/chunk-chunk-IXIZXISS.js
131.8 KiB dist/toolbar/chunk-chunk-T5KY5WYR.js
71.3 KiB dist/toolbar/toolbar-app-SSFCPEJX.js
69.0 KiB dist/toolbar/chunk-chunk-27JL52RE.js
35.6 KiB dist/toolbar/chunk-chunk-TCF7TCWO.js
20.9 KiB dist/toolbar/chunk-chunk-NMG364H6.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 — 🔺 +4.8 KiB (+0.0%)

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

Total: 1442.25 MiB · 🔺 +4.8 KiB (+0.0%)

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