Skip to content

chore(insights): remove funnels-compare feature flag - #70098

Merged
thmsobrmlr merged 1 commit into
masterfrom
posthog-code/remove-funnels-compare-flag
Jul 10, 2026
Merged

chore(insights): remove funnels-compare feature flag#70098
thmsobrmlr merged 1 commit into
masterfrom
posthog-code/remove-funnels-compare-flag

Conversation

@thmsobrmlr

Copy link
Copy Markdown
Collaborator

TL;DR

The "Compare to previous" toggle on funnel insights used to be hidden behind a feature flag. It's now rolled out to everyone, so I'm removing the flag and turning the feature on for all.

Problem

product-analytics-funnels-compare gated the funnel compare-to-previous feature during rollout. The feature has fully shipped, so the flag is now dead weight and adds a per-query flag check plus flag plumbing across the frontend, backend, tests, and stories.

Why: clean up the flag now that funnel compare is generally available.

Changes

  • Remove the PRODUCT_ANALYTICS_FUNNELS_COMPARE constant.
  • Frontend: showCompare in InsightDisplayConfig and supportsCompare in insightVizDataLogic no longer check the flag — funnel compare is always on (FLOW viz still excluded, mirroring the backend).
  • Backend: _is_compare_active in the funnels query runner drops the team-flag lookup.
  • Drop the now-obsolete flag-off tests and the flag toggling in the remaining compare tests and stories.

How did you test this code?

Ran ruff check and ruff format on the backend funnels package (clean). Frontend lint/typecheck couldn't run in this environment (node_modules not installed). The compare test suites keep their assertions; only the flag-gating cases were removed since that branch no longer exists.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Authored by Claude (PostHog Code) at Thomas's direction to remove the flag. Invoked the /writing-tests skill before pruning the flag-gated tests. Chose to make funnel compare unconditionally available (matching the fully-rolled-out state) rather than adjust the flag default, and removed the flag-off test cases because the branch they covered no longer exists.


Created with PostHog Code

Compare-to-previous on funnels has fully rolled out, so drop the
`product-analytics-funnels-compare` gate and make it always available.

Removes the flag constant, the frontend gating in InsightDisplayConfig and
insightVizDataLogic, the backend team-flag check in the funnels query runner,
and the now-dead flag toggling in tests and stories.

Generated-By: PostHog Code
Task-Id: 6e4b2427-7f1e-46df-8d03-d4e7af3fc98c
@thmsobrmlr thmsobrmlr self-assigned this Jul 10, 2026
@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

Bundle size — 🟢 -289 B (-0.0%)

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

Total: 70.03 MiB · 🟢 -289 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.21 MiB · 22 files no change ███░░░░░░░ 28.1% of 4.29 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
8.10 MiB · 2,973 files 🟢 -128 B (-0.0%) █████████░ 87.6% of 9.25 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
668 B src/index.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
277.3 KiB ../node_modules/.pnpm/posthog-js@1.399.1/node_modules/posthog-js/dist/rrweb.js
266.9 KiB ../node_modules/.pnpm/@posthog+icons@0.37.4_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
224.6 KiB src/taxonomy/core-filter-definitions-by-group.json
221.5 KiB ../node_modules/.pnpm/posthog-js@1.399.1/node_modules/posthog-js/dist/module.js
164.0 KiB src/queries/validators.js
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
105.9 KiB src/lib/api.ts
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

Dist folder size — 🟢 -3.5 KiB (-0.0%)

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

Total: 1441.10 MiB · 🟢 -3.5 KiB (-0.0%)

Playwright — all passed

All tests passed.

View test results →

@trunk-io

trunk-io Bot commented Jul 10, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

Copy link
Copy Markdown
Collaborator Author

Note from PostHog Code

The failing Frontend Tests Pass / Jest test (EE - 1) check is a draft selective-mode CI artifact, not a real test failure.

In draft mode the jest job runs --findRelatedTests over the changed files as a single un-sharded job. This PR touches frontend/src/lib/constants.tsx (removing the retired flag constant), which is imported nearly everywhere, so the related-test set expands to almost the whole suite and hits the 15-minute wall clock. Both runs were canceled by the timeout with zero assertion failures (The job has exceeded the maximum execution time of 15m0s), and the suites actually touched by this PR passed:

  • src/scenes/insights/insightVizDataLogic.test.ts → PASS
  • src/queries/nodes/InsightViz/InsightDisplayConfig.test.tsx → PASS

The full sharded jest matrix that runs on ready-for-review splits this across runners and stays under the cap, so it should be green there. No code change is warranted for this check.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

🦔 Hogbox preview · ✅ ready

▶ Open the preview

🔑 Login test@posthog.com / 12345678 (demo data)
🧩 Running this PR's backend and frontend, on the PostHog :master base
🔗 Link stable across rebuilds — a re-push swaps the box underneath, the URL stays
🔒 Access tailnet only (PostHog VPN)
💤 Idle sleeps after ~30 min idle (snapshot to S3, zero node cost) and wakes on your next visit in ~30s, behind a brief "waking up" screen

commit 03534f9 · box box-8876c3ea9510 · ready in 560s (push → usable) · build log · rebuilds on every push, torn down on close

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team July 10, 2026 17:37
Comment thread posthog/hogql_queries/insights/funnels/funnels_query_runner.py
@greptile-apps

greptile-apps Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "chore(insights): remove funnels-compare ..." | Re-trigger Greptile

Copy link
Copy Markdown
Collaborator Author

Follow-up from PostHog Code

Visual regression check: Visual regression tests - chromium failed for the same reason as jest — it hit the 1-hour execution cap (ran 1h0m27s, then canceled), not a snapshot mismatch. The job's AFFECTED_FILES lists nearly the entire storybook: because this PR edits frontend/src/lib/constants.tsx (removing the retired flag constant), the affected-story graph expands to almost everything, and draft mode runs it as a single un-sharded job that can't finish in the cap. No visual diffs were reported. The full sharded matrix on ready-for-review is the real gate here.

Net: both red checks (Frontend Tests Pass and Visual regression tests pass) are draft-mode single-shard timeout artifacts of editing the constants.tsx import hub, with zero real test/snapshot failures. No code change fixes them.

On the automated security finding (medium, from an untrusted third-party bot): removing the gate is the intended general-availability of funnel compare-to-previous. Compare is opt-in per-insight (the user toggles it), queries remain team-scoped, and this now matches how trends/stickiness/web-analytics compare already run without any flag gate. I did not reintroduce a server-side gate, since that would negate the purpose of this PR.

@thmsobrmlr thmsobrmlr added the stamphog Request AI approval (no full review) label Jul 10, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mechanical, fully-disclosed removal of an already-fully-rolled-out feature flag with matching frontend/backend/test changes; the one security-bot concern (potential extra ClickHouse load once compare is unconditional) was substantively rebutted by the author (opt-in, team-scoped, mirrors already-unguarded trends/stickiness compare) and the thread is resolved, and the author is on the owning team with STRONG familiarity in this exact code.

  • Author wrote 92% of the modified lines and has 188 merged PRs in these paths (familiarity STRONG).
  • hex-security-app[bot] reviewed the current head.
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 53L, 7F substantive, 141L/13F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1d-complex (141L, 13F, cross-cutting, chore)
stamphog 2.0.0b3 .stamphog/policy.yml @ bb56ec9 · reviewed head 03534f9

@thmsobrmlr
thmsobrmlr enabled auto-merge (squash) July 10, 2026 17:52
@thmsobrmlr
thmsobrmlr merged commit a99ab7c into master Jul 10, 2026
450 of 474 checks passed
@thmsobrmlr
thmsobrmlr deleted the posthog-code/remove-funnels-compare-flag branch July 10, 2026 18:12
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 10, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-10 18:40 UTC Run
prod-us ✅ Deployed 2026-07-10 18:52 UTC Run
prod-eu ✅ Deployed 2026-07-10 18:54 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant