Repository: PostHog/posthog
Pull request: Exclude action events from correlation results
## Changes

Fixes https://github.com/PostHog/posthog/issues/7164

## How did you test this code?

Unit test


Problem statement:
Exclude underlying events from correlation with actions
## Bug description

When running a Correlation Analysis using actions, you always (or almost always) see the underlying events that comprise the action(s) with large correlation likelihoods. This is obviously pretty useless as we're using the same event to say there's a high correlation likelihood.

## Expected behavior

If I use an action in a funnel, no underlying events for that action should show up in the Correlation analysis results.


## How to reproduce

1. Create a funnel using actions as steps.
2. See underlying events pop up in Correlation analysis with huge correlation coefficients.

## Environment

- [X] PostHog Cloud
- [X] self-hosted PostHog (ClickHouse-based), version/commit: `1.30.0`
- [ ] self-hosted PostHog (Postgres-based, legacy), version/commit: _please provide_

## Additional context



#### *Thank you* for your bug report – we love squashing them!

Hints from issue discussion:

Commit message: update snapshots

Patch to review:
diff --git a/ee/clickhouse/queries/funnels/funnel_correlation.py b/ee/clickhouse/queries/funnels/funnel_correlation.py
index fccb5c8c842b8..df794c2855f87 100644
--- a/ee/clickhouse/queries/funnels/funnel_correlation.py
+++ b/ee/clickhouse/queries/funnels/funnel_correlation.py
@@ -25,7 +25,12 @@
 from ee.clickhouse.queries.groups_join_query import GroupsJoinQuery
 from ee.clickhouse.queries.person_query import ClickhousePersonQuery
 from ee.clickhouse.sql.person import GET_TEAM_PERSON_DISTINCT_IDS
-from posthog.constants import AUTOCAPTURE_EVENT, TREND_FILTER_TYPE_EVENTS, FunnelCorrelationType
+from posthog.constants import (
+    AUTOCAPTURE_EVENT,
+    TREND_FILTER_TYPE_ACTIONS,
+    TREND_FILTER_TYPE_EVENTS,
+    FunnelCorrelationType,
+)
 from posthog.models import Filter, Team
 from posthog.models.entity import Entity
 from posthog.models.filters import Filter
@@ -217,7 +222,7 @@ def get_event_query(self) -> Tuple[str, Dict[str, Any]]:
         """
         params = {
             **funnel_persons_params,
-            "funnel_step_names": [entity.id for entity in self._filter.events],
+            "funnel_step_names": self._get_funnel_step_names(),
             "target_step": len(self._filter.entities),
             "exclude_event_names": self._filter.correlation_event_exclude_names,
         }
@@ -296,7 +301,7 @@ def get_event_property_query(self) -> Tuple[str, Dict[str, Any]]:
         """
         params = {
             **funnel_persons_params,
-            "funnel_step_names": [entity.id for entity in self._filter.events],
+            "funnel_step_names": self._get_funnel_step_names(),
             "target_step": len(self._filter.entities),
             "event_names": self._filter.correlation_event_names,
             "exclude_property_names": self._filter.correlation_event_exclude_property_names,
@@ -504,6 +509,18 @@ def _get_properties_prop_clause(self):
                 person_property_params,
             )
 
+    def _get_funnel_step_names(self):
+        events = set()
+        for entity in self._filter.entities:
+            if entity.type == TREND_FILTER_TYPE_ACTIONS:
+                action = entity.get_action()
+                for action_step in action.steps.all():
+                    events.add(action_step.event)
+            else:
+                events.add(entity.id)
+
+        return sorted(list(events))
+
     def _run(self) -> Tuple[List[EventOddsRatio], bool]:
         """
         Run the diagnose query.
diff --git a/ee/clickhouse/queries/funnels/test/__snapshots__/test_funnel_correlation.ambr b/ee/clickhouse/queries/funnels/test/__snapshots__/test_funnel_correlation.ambr
index 3586c57a2f7c5..ad3e0b47eca82 100644
--- a/ee/clickhouse/queries/funnels/test/__snapshots__/test_funnel_correlation.ambr
+++ b/ee/clickhouse/queries/funnels/test/__snapshots__/test_funnel_correlation.ambr
@@ -1054,7 +1054,7 @@
        toDateTime('2020-01-14 23:59:59') AS date_to,
        toDateTime('2020-01-01 00:00:00') AS date_from,
        2 AS target_step,
-       ['user signed up', 'paid'] as funnel_step_names
+       ['paid', 'user signed up'] as funnel_step_names
   SELECT concat(event_name, '::', prop.1, '::', prop.2) as name,
          countDistinctIf(person_id, steps = target_step) as success_count,
          countDistinctIf(person_id, steps <> target_step) as failure_count
@@ -1167,7 +1167,7 @@
        toDateTime('2020-01-14 23:59:59') AS date_to,
        toDateTime('2020-01-01 00:00:00') AS date_from,
        2 AS target_step,
-       ['user signed up', 'paid'] as funnel_step_names
+       ['paid', 'user signed up'] as funnel_step_names
   SELECT concat(event_name, '::', prop.1, '::', prop.2) as name,
          countDistinctIf(person_id, steps = target_step) as success_count,
          countDistinctIf(person_id, steps <> target_step) as failure_count
@@ -1198,6 +1198,130 @@
   FROM funnel_people AS person
   '
 ---
+# name: TestClickhouseFunnelCorrelation.test_funnel_correlation_with_events_and_actions
+  '
+  WITH funnel_people as
+    (SELECT aggregation_target as person_id,
+            timestamp,
+            steps,
+            final_timestamp,
+            first_timestamp
+     FROM
+       (SELECT aggregation_target,
+               steps,
+               avg(step_1_conversion_time) step_1_average_conversion_time_inner,
+               median(step_1_conversion_time) step_1_median_conversion_time_inner ,
+               argMax(latest_0, steps) as timestamp,
+               argMax(latest_1, steps) as final_timestamp,
+               argMax(latest_0, steps) as first_timestamp
+        FROM
+          (SELECT aggregation_target,
+                  steps,
+                  max(steps) over (PARTITION BY aggregation_target) as max_steps,
+                                  step_1_conversion_time ,
+                                  latest_0,
+                                  latest_1,
+                                  latest_0
+           FROM
+             (SELECT *,
+                     if(latest_0 < latest_1
+                        AND latest_1 <= latest_0 + INTERVAL 14 DAY, 2, 1) AS steps ,
+                     if(isNotNull(latest_1)
+                        AND latest_1 <= latest_0 + INTERVAL 14 DAY, dateDiff('second', toDateTime(latest_0), toDateTime(latest_1)), NULL) step_1_conversion_time
+              FROM
+                (SELECT aggregation_target,
+                        timestamp,
+                        step_0,
+                        latest_0,
+                        step_1,
+                        min(latest_1) over (PARTITION by aggregation_target
+                                            ORDER BY timestamp DESC ROWS BETWEEN UNBOUNDED PRECEDING AND 0 PRECEDING) latest_1
+                 FROM
+                   (SELECT aggregation_target,
+                           timestamp,
+                           if(((event = 'user signed up'
+                                AND has(['val'], trim(BOTH '"'
+                                                      FROM JSONExtractRaw(properties, 'key')))
+                                AND team_id = 2)) , 1, 0) as step_0,
+                           if(step_0 = 1, timestamp, null) as latest_0,
+                           if(((event = 'paid'
+                                AND has(['val'], trim(BOTH '"'
+                                                      FROM JSONExtractRaw(properties, 'key')))
+                                AND team_id = 2)) , 1, 0) as step_1,
+                           if(step_1 = 1, timestamp, null) as latest_1
+                    FROM
+                      (SELECT e.event as event,
+                              e.team_id as team_id,
+                              e.distinct_id as distinct_id,
+                              e.timestamp as timestamp,
+                              pdi.person_id as aggregation_target,
+                              e.properties as properties
+                       FROM events e
+                       INNER JOIN
+                         (SELECT distinct_id,
+                                 argMax(person_id, _timestamp) as person_id
+                          FROM
+                            (SELECT distinct_id,
+                                    person_id,
+                                    max(_timestamp) as _timestamp
+                             FROM person_distinct_id
+                             WHERE team_id = 2
+                             GROUP BY person_id,
+                                      distinct_id,
+                                      team_id
+                             HAVING max(is_deleted) = 0)
+                          GROUP BY distinct_id) AS pdi ON events.distinct_id = pdi.distinct_id
+                       WHERE team_id = 2
+                         AND event IN ['paid', 'user signed up', 'user signed up', 'paid']
+                         AND timestamp >= '2020-01-01 00:00:00'
+                         AND timestamp <= '2020-01-14 23:59:59' ) events
+                    WHERE (step_0 = 1
+                           OR step_1 = 1) ))
+              WHERE step_0 = 1 SETTINGS allow_experimental_window_functions = 1 ))
+        GROUP BY aggregation_target,
+                 steps
+        HAVING steps = max_steps SETTINGS allow_experimental_window_functions = 1)
+     WHERE steps IN [1, 2]
+     ORDER BY aggregation_target
+     OFFSET 0 SETTINGS allow_experimental_window_functions = 1),
+       toDateTime('2020-01-14 23:59:59') AS date_to,
+       toDateTime('2020-01-01 00:00:00') AS date_from,
+       2 AS target_step,
+       ['paid', 'user signed up'] as funnel_step_names
+  SELECT event.event AS name,
+         countDistinctIf(person.person_id, person.steps = target_step) AS success_count,
+         countDistinctIf(person.person_id, person.steps <> target_step) AS failure_count
+  FROM events AS event
+  JOIN
+    (SELECT distinct_id,
+            argMax(person_id, _timestamp) as person_id
+     FROM
+       (SELECT distinct_id,
+               person_id,
+               max(_timestamp) as _timestamp
+        FROM person_distinct_id
+        WHERE team_id = 2
+        GROUP BY person_id,
+                 distinct_id,
+                 team_id
+        HAVING max(is_deleted) = 0)
+     GROUP BY distinct_id) AS pdi ON pdi.distinct_id = events.distinct_id
+  JOIN funnel_people AS person ON pdi.person_id = person.person_id
+  WHERE event.timestamp >= date_from
+    AND event.timestamp < date_to
+    AND event.team_id = 2
+    AND event.timestamp > person.first_timestamp
+    AND event.timestamp < COALESCE(person.final_timestamp, person.first_timestamp + INTERVAL 14 DAY, date_to)
+    AND event.event NOT IN funnel_step_names
+    AND event.event NOT IN []
+  GROUP BY name
+  UNION ALL
+  SELECT 'Total_Values_In_Query' as name,
+         countDistinctIf(person.person_id, person.steps = target_step) AS success_count,
+         countDistinctIf(person.person_id, person.steps <> target_step) AS failure_count
+  FROM funnel_people AS person
+  '
+---
 # name: TestClickhouseFunnelCorrelation.test_funnel_correlation_with_events_and_groups
   '
   WITH funnel_people as
@@ -1280,7 +1404,7 @@
        toDateTime('2020-01-14 23:59:59') AS date_to,
        toDateTime('2020-01-01 00:00:00') AS date_from,
        2 AS target_step,
-       ['user signed up', 'paid'] as funnel_step_names
+       ['paid', 'user signed up'] as funnel_step_names
   SELECT event.event AS name,
          countDistinctIf(person.person_id, person.steps = target_step) AS success_count,
          countDistinctIf(person.person_id, person.steps <> target_step) AS failure_count
@@ -1393,7 +1517,7 @@
        toDateTime('2020-01-14 23:59:59') AS date_to,
        toDateTime('2020-01-01 00:00:00') AS date_from,
        2 AS target_step,
-       ['user signed up', 'paid'] as funnel_step_names
+       ['paid', 'user signed up'] as funnel_step_names
   SELECT event.event AS name,
          countDistinctIf(person.person_id, person.steps = target_step) AS success_count,
          countDistinctIf(person.person_id, person.steps <> target_step) AS failure_count
diff --git a/ee/clickhouse/queries/funnels/test/test_funnel_correlation.py b/ee/clickhouse/queries/funnels/test/test_funnel_correlation.py
index 420355a8e3407..10028225e0382 100644
--- a/ee/clickhouse/queries/funnels/test/test_funnel_correlation.py
+++ b/ee/clickhouse/queries/funnels/test/test_funnel_correlation.py
@@ -10,6 +10,8 @@
 from ee.clickhouse.queries.funnels.funnel_correlation_persons import FunnelCorrelationPersons
 from ee.clickhouse.util import ClickhouseTestMixin, snapshot_clickhouse_queries
 from posthog.constants import INSIGHT_FUNNELS
+from posthog.models.action import Action
+from posthog.models.action_step import ActionStep
 from posthog.models.element import Element
 from posthog.models.filters import Filter
 from posthog.models.group_type_mapping import GroupTypeMapping
@@ -18,6 +20,15 @@
 from posthog.test.base import APIBaseTest, test_with_materialized_columns
 
 
+def _create_action(**kwargs):
+    team = kwargs.pop("team")
+    name = kwargs.pop("name")
+    properties = kwargs.pop("properties", {})
+    action = Action.objects.create(team=team, name=name)
+    ActionStep.objects.create(action=action, event=name, properties=properties)
+    return action
+
+
 def _create_person(**kwargs):
     person = Person.objects.create(**kwargs)
     return Person(id=person.uuid, uuid=person.uuid)
@@ -160,6 +171,104 @@ def test_basic_funnel_correlation_with_events(self):
         self.assertEqual(len(self._get_people_for_event(filter, "negatively_related", success=False)), 5)
         self.assertEqual(len(self._get_people_for_event(filter, "negatively_related")), 0)
 
+    @snapshot_clickhouse_queries
+    def test_funnel_correlation_with_events_and_actions(self):
+
+        for i in range(10):
+            _create_person(distinct_ids=[f"user_{i}"], team_id=self.team.pk)
+            _create_event(
+                team=self.team,
+                event="user signed up",
+                distinct_id=f"user_{i}",
+                timestamp="2020-01-02T14:00:00Z",
+                properties={"key": "val"},
+            )
+            # same event, but missing property, so not part of action.
+            _create_event(
+                team=self.team, event="user signed up", distinct_id=f"user_{i}", timestamp="2020-01-02T14:10:00Z",
+            )
+            if i % 2 == 0:
+                _create_event(
+                    team=self.team,
+                    event="positively_related",
+                    distinct_id=f"user_{i}",
+                    timestamp="2020-01-03T14:00:00Z",
+                )
+            _create_event(
+                team=self.team,
+                event="paid",
+                distinct_id=f"user_{i}",
+                timestamp="2020-01-04T14:00:00Z",
+                properties={"key": "val"},
+            )
+
+        for i in range(10, 20):
+            _create_person(distinct_ids=[f"user_{i}"], team_id=self.team.pk)
+            _create_event(
+                team=self.team,
+                event="user signed up",
+                distinct_id=f"user_{i}",
+                timestamp="2020-01-02T14:00:00Z",
+                properties={"key": "val"},
+            )
+            if i % 2 == 0:
+                _create_event(
+                    team=self.team,
+                    event="negatively_related",
+                    distinct_id=f"user_{i}",
+                    timestamp="2020-01-03T14:00:00Z",
+                )
+
+        sign_up_action = _create_action(
+            name="user signed up",
+            team=self.team,
+            properties=[{"key": "key", "type": "event", "value": ["val"], "operator": "exact"}],
+        )
+        paid_action = _create_action(
+            name="paid",
+            team=self.team,
+            properties=[{"key": "key", "type": "event", "value": ["val"], "operator": "exact"}],
+        )
+        filters = {
+            "events": [],
+            "actions": [{"id": sign_up_action.id, "order": 0}, {"id": paid_action.id, "order": 1},],
+            "insight": INSIGHT_FUNNELS,
+            "date_from": "2020-01-01",
+            "date_to": "2020-01-14",
+            "funnel_correlation_type": "events",
+        }
+
+        filter = Filter(data=filters)
+        correlation = FunnelCorrelation(filter, self.team)
+        result = correlation._run()[0]
+
+        odds_ratios = [item.pop("odds_ratio") for item in result]  # type: ignore
+        expected_odds_ratios = [11, 1 / 11]
+
+        for odds, expected_odds in zip(odds_ratios, expected_odds_ratios):
+            self.assertAlmostEqual(odds, expected_odds)
+
+        #  missing user signed up and paid from result set, as expected
+        self.assertEqual(
+            result,
+            [
+                {
+                    "event": "positively_related",
+                    "success_count": 5,
+                    "failure_count": 0,
+                    # "odds_ratio": 11.0,
+                    "correlation_type": "success",
+                },
+                {
+                    "event": "negatively_related",
+                    "success_count": 0,
+                    "failure_count": 5,
+                    # "odds_ratio": 1 / 11,
+                    "correlation_type": "failure",
+                },
+            ],
+        )
+
     @snapshot_clickhouse_queries
     def test_funnel_correlation_with_events_and_groups(self):
         GroupTypeMapping.objects.create(team=self.team, group_type="organization", group_type_index=0)


Code review comments:
diff_hunk: @@ -160,6 +171,104 @@ def test_basic_funnel_correlation_with_events(self):
         self.assertEqual(len(self._get_people_for_event(filter, "negatively_related", success=False)), 5)
         self.assertEqual(len(self._get_people_for_event(filter, "negatively_related")), 0)
 
+    @snapshot_clickhouse_queries
+    def test_funnel_correlation_with_events_and_actions(self):
+
+        for i in range(10):
+            _create_person(distinct_ids=[f"user_{i}"], team_id=self.team.pk)
+            _create_event(
path: ee/clickhouse/queries/funnels/test/test_funnel_correlation.py
text: @user1:
non-blocking as is consistent with other tests in the file.

Can we use the `ee.clickhouse.test.test_journeys.journeys_for` to construct these scenarios?

diff_hunk: @@ -160,6 +171,104 @@ def test_basic_funnel_correlation_with_events(self):
         self.assertEqual(len(self._get_people_for_event(filter, "negatively_related", success=False)), 5)
         self.assertEqual(len(self._get_people_for_event(filter, "negatively_related")), 0)
 
+    @snapshot_clickhouse_queries
+    def test_funnel_correlation_with_events_and_actions(self):
+
+        for i in range(10):
path: ee/clickhouse/queries/funnels/test/test_funnel_correlation.py
text: @user1:
Is there a reason for so many events? If not, I would suggest we go with the minimum reproducible test case for this specific test.

@author:
Fair, will update!